| Message ID | 20200103064407.19861-2-michael.kao@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Mediatek thermal dirver and dtsi | expand |
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 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. > > > + 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 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 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.
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>; + 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>; + thermal_calibration: calib@180 { + reg = <0x180 0xc>; + }; }; mfgcfg: syscon@13000000 {