| Message ID | 20200103064407.19861-1-michael.kao@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add Mediatek thermal dirver and dtsi | expand |
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>
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>; [ ... ]
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>; > }; > }; > >
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.
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 { >
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
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
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>; > }; > }; > >
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.