* [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node @ 2018-06-28 21:09 Matthias Kaehlcke 2018-06-28 21:09 ` [PATCH 2/3] arm64: dts: qcom: pm8998: Add spmi-temp-alarm node Matthias Kaehlcke ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Matthias Kaehlcke @ 2018-06-28 21:09 UTC (permalink / raw) To: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon Cc: linux-soc, linux-arm-msm, linux-arm-kernel, linux-kernel, David Collins, Douglas Anderson, Stephen Boyd, Matthias Kaehlcke The node is empty for now. It is needed to allow other DT snippets to add thermal zone entries. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- Sorry if you received this twice, lists were missing in cc in the first attempt ... arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 00722b533a92..968390fb0933 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -9,6 +9,7 @@ #include <dt-bindings/clock/qcom,rpmh.h> #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> +#include <dt-bindings/thermal/thermal.h> / { interrupt-parent = <&intc>; @@ -1087,4 +1088,7 @@ }; }; }; + + thermal_zones: thermal-zones { + }; }; -- 2.18.0.rc2.346.g013aa6912e-goog ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] arm64: dts: qcom: pm8998: Add spmi-temp-alarm node 2018-06-28 21:09 [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Matthias Kaehlcke @ 2018-06-28 21:09 ` Matthias Kaehlcke 2018-06-28 21:09 ` [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone Matthias Kaehlcke 2018-06-28 22:52 ` [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Doug Anderson 2 siblings, 0 replies; 16+ messages in thread From: Matthias Kaehlcke @ 2018-06-28 21:09 UTC (permalink / raw) To: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon Cc: linux-soc, linux-arm-msm, linux-arm-kernel, linux-kernel, David Collins, Douglas Anderson, Stephen Boyd, Matthias Kaehlcke This adds the spmi-temp-alarm node to pm8998 based on the examples in the bindings. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- arch/arm64/boot/dts/qcom/pm8998.dtsi | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi index 92bed1e7d4bb..f2d39074ed74 100644 --- a/arch/arm64/boot/dts/qcom/pm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi @@ -44,6 +44,12 @@ <0 0xd9 0 IRQ_TYPE_NONE>; }; + pm8998_temp: qcom,temp-alarm@2400 { + compatible = "qcom,spmi-temp-alarm"; + reg = <0x2400 0x100>; + interrupts = <0x0 0x24 0x0 IRQ_TYPE_EDGE_RISING>; + #thermal-sensor-cells = <0>; + }; }; pm8998_lsid1: pmic@1 { -- 2.18.0.rc2.346.g013aa6912e-goog ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-06-28 21:09 [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Matthias Kaehlcke 2018-06-28 21:09 ` [PATCH 2/3] arm64: dts: qcom: pm8998: Add spmi-temp-alarm node Matthias Kaehlcke @ 2018-06-28 21:09 ` Matthias Kaehlcke 2018-06-28 22:58 ` Doug Anderson 2018-06-28 22:52 ` [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Doug Anderson 2 siblings, 1 reply; 16+ messages in thread From: Matthias Kaehlcke @ 2018-06-28 21:09 UTC (permalink / raw) To: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon Cc: linux-soc, linux-arm-msm, linux-arm-kernel, linux-kernel, David Collins, Douglas Anderson, Stephen Boyd, Matthias Kaehlcke Add pm8998 thermal zone based on the examples in the spmi-temp-alarm bindings. Note: devices with the pm8998 need to have a 'thermal-zones' node (which may be empty) with a label 'thermal_zones'. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- arch/arm64/boot/dts/qcom/pm8998.dtsi | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi index f2d39074ed74..d85ceb4f976b 100644 --- a/arch/arm64/boot/dts/qcom/pm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi @@ -3,6 +3,7 @@ #include <dt-bindings/spmi/spmi.h> #include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/thermal/thermal.h> &spmi_bus { pm8998_lsid0: pmic@0 { @@ -59,3 +60,30 @@ #size-cells = <0>; }; }; + +&thermal_zones { + pm8998 { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&pm8998_temp>; + + trips { + passive { + temperature = <1050000>; + hysteresis = <2000>; + type = "passive"; + }; + alert { + temperature = <125000>; + hysteresis = <2000>; + type = "hot"; + }; + crit { + temperature = <145000>; + hysteresis = <2000>; + type = "critical"; + }; + }; + }; +}; -- 2.18.0.rc2.346.g013aa6912e-goog ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-06-28 21:09 ` [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone Matthias Kaehlcke @ 2018-06-28 22:58 ` Doug Anderson 2018-06-29 18:51 ` Matthias Kaehlcke 0 siblings, 1 reply; 16+ messages in thread From: Doug Anderson @ 2018-06-28 22:58 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, David Collins, Stephen Boyd Hi, On Thu, Jun 28, 2018 at 2:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > Add pm8998 thermal zone based on the examples in the spmi-temp-alarm > bindings. > > Note: devices with the pm8998 need to have a 'thermal-zones' node (which > may be empty) with a label 'thermal_zones'. > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > --- > arch/arm64/boot/dts/qcom/pm8998.dtsi | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) Do you know if this patch actually does anything since you didn't define a cooling-maps? Hopefully at least the critical shuts things down? Do you have any idea how we'll eventually want to specify a cooling-maps? Are we just going to assume we're included by an sdm845 device and refer to the big/little CPU phandles? > diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi > index f2d39074ed74..d85ceb4f976b 100644 > --- a/arch/arm64/boot/dts/qcom/pm8998.dtsi > +++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi > @@ -3,6 +3,7 @@ > > #include <dt-bindings/spmi/spmi.h> > #include <dt-bindings/interrupt-controller/irq.h> > +#include <dt-bindings/thermal/thermal.h> > > &spmi_bus { > pm8998_lsid0: pmic@0 { > @@ -59,3 +60,30 @@ > #size-cells = <0>; > }; > }; > + > +&thermal_zones { As per comments in patch #1, don't rely on a label. Just put your stuff in a top-level "thermal-zones" node. > + pm8998 { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&pm8998_temp>; > + > + trips { > + passive { IMO you should proactively put a label on these trips even if there's no cooling device yet. It's expected that at some point someone will add a cooling device and refer to them, right? -Doug ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-06-28 22:58 ` Doug Anderson @ 2018-06-29 18:51 ` Matthias Kaehlcke 2018-06-29 21:29 ` David Collins 0 siblings, 1 reply; 16+ messages in thread From: Matthias Kaehlcke @ 2018-06-29 18:51 UTC (permalink / raw) To: Doug Anderson Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, David Collins, Stephen Boyd On Thu, Jun 28, 2018 at 03:58:41PM -0700, Doug Anderson wrote: > Hi, > > On Thu, Jun 28, 2018 at 2:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > > Add pm8998 thermal zone based on the examples in the spmi-temp-alarm > > bindings. > > > > Note: devices with the pm8998 need to have a 'thermal-zones' node (which > > may be empty) with a label 'thermal_zones'. > > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > --- > > arch/arm64/boot/dts/qcom/pm8998.dtsi | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > Do you know if this patch actually does anything since you didn't > define a cooling-maps? Hopefully at least the critical shuts things > down? I need to do some additional testing, currently waiting to get the heat gun back ... I would expect the critical trip point to shut the system down, though I'm not sure whether the HW temperature watchdog wouldn't cut power before that. The documentation I have access to contains some register descriptions, but isn't very verbose about the overall behavior and from the driver code that's also not really clear to me. The driver "disables software override of stage 2 and 3 shutdowns" which make me guess that a hardware shutdown kicks in at stage 2 (135°C ?). This would be roughly in line with a system reset I observed in an earlier test at a temperature > 125°C. If that's correct the trip points need to be revisited. Maybe David Collins who recently extended the driver to add support for GEN2 PMIC peripherals can provide more details. > Do you have any idea how we'll eventually want to specify a > cooling-maps? Are we just going to assume we're included by an sdm845 > device and refer to the big/little CPU phandles? No clear idea on my side at this point, but limiting CPU frequencies seems likely, potentially also devfreq devices. > > diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi > > index f2d39074ed74..d85ceb4f976b 100644 > > --- a/arch/arm64/boot/dts/qcom/pm8998.dtsi > > +++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi > > @@ -3,6 +3,7 @@ > > > > #include <dt-bindings/spmi/spmi.h> > > #include <dt-bindings/interrupt-controller/irq.h> > > +#include <dt-bindings/thermal/thermal.h> > > > > &spmi_bus { > > pm8998_lsid0: pmic@0 { > > @@ -59,3 +60,30 @@ > > #size-cells = <0>; > > }; > > }; > > + > > +&thermal_zones { > > As per comments in patch #1, don't rely on a label. Just put your > stuff in a top-level "thermal-zones" node. ack > > + pm8998 { > > + polling-delay-passive = <250>; > > + polling-delay = <1000>; > > + > > + thermal-sensors = <&pm8998_temp>; > > + > > + trips { > > + passive { > > IMO you should proactively put a label on these trips even if there's > no cooling device yet. It's expected that at some point someone will > add a cooling device and refer to them, right? ok ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-06-29 18:51 ` Matthias Kaehlcke @ 2018-06-29 21:29 ` David Collins 2018-06-29 23:54 ` Matthias Kaehlcke 0 siblings, 1 reply; 16+ messages in thread From: David Collins @ 2018-06-29 21:29 UTC (permalink / raw) To: Matthias Kaehlcke, Doug Anderson Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd Hello Matthias, On 06/29/2018 11:51 AM, Matthias Kaehlcke wrote: > On Thu, Jun 28, 2018 at 03:58:41PM -0700, Doug Anderson wrote: >> Hi, >> >> On Thu, Jun 28, 2018 at 2:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote: >>> Add pm8998 thermal zone based on the examples in the spmi-temp-alarm >>> bindings. >>> >>> Note: devices with the pm8998 need to have a 'thermal-zones' node (which >>> may be empty) with a label 'thermal_zones'. >>> >>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> >>> --- >>> arch/arm64/boot/dts/qcom/pm8998.dtsi | 28 ++++++++++++++++++++++++++++ >>> 1 file changed, 28 insertions(+) >> >> Do you know if this patch actually does anything since you didn't >> define a cooling-maps? Hopefully at least the critical shuts things >> down? > > I need to do some additional testing, currently waiting to get the > heat gun back ... > > I would expect the critical trip point to shut the system down, though > I'm not sure whether the HW temperature watchdog wouldn't cut power > before that. The documentation I have access to contains some register > descriptions, but isn't very verbose about the overall behavior and > from the driver code that's also not really clear to me. The driver > "disables software override of stage 2 and 3 shutdowns" which make me > guess that a hardware shutdown kicks in at stage 2 (135°C ?). This > would be roughly in line with a system reset I observed in an earlier > test at a temperature > 125°C. If that's correct the trip points need > to be revisited. > > Maybe David Collins who recently extended the driver to add support > for GEN2 PMIC peripherals can provide more details. The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns off peripherals within the PMIC that are expected to draw significant current. The set of peripherals included varies between PMICs. This partial shutdown will occur simultaneously with the triggering of an interrupt to the APPS processor that informs the qcom-spmi-temp-alarm driver that an over-temperature threshold has been crossed. The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown upon hitting over-temperature stage 3 (145 C). Software won't receive an interrupt in this case because all power is cut. If you are not specifying an ADC channel for the qcom-spmi-temp-alarm device (which would allow for polling of the real-time PMIC die temperature), then notifications about stage 0 -> 1 and 1 -> 0 transitions (105 C) are the only time that software could take meaningful corrective action to avoid a PMIC automatic partial or full shutdown. Take care, David -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-06-29 21:29 ` David Collins @ 2018-06-29 23:54 ` Matthias Kaehlcke 2018-07-10 17:45 ` David Collins 0 siblings, 1 reply; 16+ messages in thread From: Matthias Kaehlcke @ 2018-06-29 23:54 UTC (permalink / raw) To: David Collins Cc: Doug Anderson, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: > Hello Matthias, > > On 06/29/2018 11:51 AM, Matthias Kaehlcke wrote: > > On Thu, Jun 28, 2018 at 03:58:41PM -0700, Doug Anderson wrote: > >> Hi, > >> > >> On Thu, Jun 28, 2018 at 2:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > >>> Add pm8998 thermal zone based on the examples in the spmi-temp-alarm > >>> bindings. > >>> > >>> Note: devices with the pm8998 need to have a 'thermal-zones' node (which > >>> may be empty) with a label 'thermal_zones'. > >>> > >>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > >>> --- > >>> arch/arm64/boot/dts/qcom/pm8998.dtsi | 28 ++++++++++++++++++++++++++++ > >>> 1 file changed, 28 insertions(+) > >> > >> Do you know if this patch actually does anything since you didn't > >> define a cooling-maps? Hopefully at least the critical shuts things > >> down? > > > > I need to do some additional testing, currently waiting to get the > > heat gun back ... > > > > I would expect the critical trip point to shut the system down, though > > I'm not sure whether the HW temperature watchdog wouldn't cut power > > before that. The documentation I have access to contains some register > > descriptions, but isn't very verbose about the overall behavior and > > from the driver code that's also not really clear to me. The driver > > "disables software override of stage 2 and 3 shutdowns" which make me > > guess that a hardware shutdown kicks in at stage 2 (135°C ?). This > > would be roughly in line with a system reset I observed in an earlier > > test at a temperature > 125°C. If that's correct the trip points need > > to be revisited. > > > > Maybe David Collins who recently extended the driver to add support > > for GEN2 PMIC peripherals can provide more details. > > The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial > PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns > off peripherals within the PMIC that are expected to draw significant > current. The set of peripherals included varies between PMICs. This > partial shutdown will occur simultaneously with the triggering of an > interrupt to the APPS processor that informs the qcom-spmi-temp-alarm > driver that an over-temperature threshold has been crossed. > > The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown > upon hitting over-temperature stage 3 (145 C). Software won't receive an > interrupt in this case because all power is cut. This information is very useful, thanks David! The (partial) hardware shutdown seems like a good measure of last resort, however I suppose we prefer Linux to initiate a shutdown before losing part of the peripherals (drivers might not be happy about this and probably not revover even when the temperature goes down again) or reach a full PMIC shutdown. Please let me know if there are reasons to prefer to go the hardware limits, it's also an option for device makers to overwrite these settings if they want different behavior. > If you are not specifying an ADC channel for the qcom-spmi-temp-alarm > device (which would allow for polling of the real-time PMIC die > temperature), then notifications about stage 0 -> 1 and 1 -> 0 transitions > (105 C) are the only time that software could take meaningful corrective > action to avoid a PMIC automatic partial or full shutdown. Thanks, I already experimented a a bit with this. For the record, the driver is https://patchwork.kernel.org/patch/10494771/ (this version is broken though). Cheers Matthias ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-06-29 23:54 ` Matthias Kaehlcke @ 2018-07-10 17:45 ` David Collins 2018-07-11 21:56 ` Doug Anderson 0 siblings, 1 reply; 16+ messages in thread From: David Collins @ 2018-07-10 17:45 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Doug Anderson, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd Hello Matthias, On 06/29/2018 04:54 PM, Matthias Kaehlcke wrote: > On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: ... >> The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial >> PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns >> off peripherals within the PMIC that are expected to draw significant >> current. The set of peripherals included varies between PMICs. This >> partial shutdown will occur simultaneously with the triggering of an >> interrupt to the APPS processor that informs the qcom-spmi-temp-alarm >> driver that an over-temperature threshold has been crossed. >> >> The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown >> upon hitting over-temperature stage 3 (145 C). Software won't receive an >> interrupt in this case because all power is cut. > > This information is very useful, thanks David! > > The (partial) hardware shutdown seems like a good measure of last > resort, however I suppose we prefer Linux to initiate a shutdown > before losing part of the peripherals (drivers might not be happy > about this and probably not revover even when the temperature goes > down again) or reach a full PMIC shutdown. > > Please let me know if there are reasons to prefer to go the hardware > limits, it's also an option for device makers to overwrite these > settings if they want different behavior. Disabling stage 3 automatic full PMIC shutdown at 145 C is definitely a bad idea. This exists as a last resort in order to save the hardware and ensure end user safety in case of excessive temperature even if software is locked up. Disabling stage 2 automatic partial PMIC shutdown at 125 C is not recommended as the PMIC is already outside of reasonable operating conditions and needs to take corrective action quickly. However, doing so may be acceptable if software is taking action to shut down the system immediately upon receiving the stage 2 over-temperature interrupt. Take care, David -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-07-10 17:45 ` David Collins @ 2018-07-11 21:56 ` Doug Anderson 2018-07-11 22:36 ` David Collins 0 siblings, 1 reply; 16+ messages in thread From: Doug Anderson @ 2018-07-11 21:56 UTC (permalink / raw) To: David Collins Cc: Matthias Kaehlcke, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd Hi David, On Tue, Jul 10, 2018 at 10:45 AM, David Collins <collinsd@codeaurora.org> wrote: > Hello Matthias, > > On 06/29/2018 04:54 PM, Matthias Kaehlcke wrote: >> On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: > ... >>> The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial >>> PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns >>> off peripherals within the PMIC that are expected to draw significant >>> current. The set of peripherals included varies between PMICs. This >>> partial shutdown will occur simultaneously with the triggering of an >>> interrupt to the APPS processor that informs the qcom-spmi-temp-alarm >>> driver that an over-temperature threshold has been crossed. >>> >>> The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown >>> upon hitting over-temperature stage 3 (145 C). Software won't receive an >>> interrupt in this case because all power is cut. >> >> This information is very useful, thanks David! >> >> The (partial) hardware shutdown seems like a good measure of last >> resort, however I suppose we prefer Linux to initiate a shutdown >> before losing part of the peripherals (drivers might not be happy >> about this and probably not revover even when the temperature goes >> down again) or reach a full PMIC shutdown. >> >> Please let me know if there are reasons to prefer to go the hardware >> limits, it's also an option for device makers to overwrite these >> settings if they want different behavior. > > Disabling stage 3 automatic full PMIC shutdown at 145 C is definitely a > bad idea. This exists as a last resort in order to save the hardware and > ensure end user safety in case of excessive temperature even if software > is locked up. > > Disabling stage 2 automatic partial PMIC shutdown at 125 C is not > recommended as the PMIC is already outside of reasonable operating > conditions and needs to take corrective action quickly. However, doing so > may be acceptable if software is taking action to shut down the system > immediately upon receiving the stage 2 over-temperature interrupt. Just to confirm: is it expected that at stage 2 the CPU's on the SoC should continue running even with partial PMIC shutdown enabled? It sounded to me like partial PMIC shutdown was supposed to shut down high-power rails that were not essential to the task of performing an orderly shutdown. I think Matthias was seeing that when he reached stage 2 and partial PMIC shutdown happened that the system was just falling on the floor. ...maybe we just have things configured incorrectly? -Doug ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-07-11 21:56 ` Doug Anderson @ 2018-07-11 22:36 ` David Collins 2018-07-11 22:43 ` Doug Anderson 0 siblings, 1 reply; 16+ messages in thread From: David Collins @ 2018-07-11 22:36 UTC (permalink / raw) To: Doug Anderson Cc: Matthias Kaehlcke, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd Hello Doug, > On Tue, Jul 10, 2018 at 10:45 AM, David Collins <collinsd@codeaurora.org> wrote: >> On 06/29/2018 04:54 PM, Matthias Kaehlcke wrote: >>> On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: >> ... >>>> The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial >>>> PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns >>>> off peripherals within the PMIC that are expected to draw significant >>>> current. The set of peripherals included varies between PMICs. This >>>> partial shutdown will occur simultaneously with the triggering of an >>>> interrupt to the APPS processor that informs the qcom-spmi-temp-alarm >>>> driver that an over-temperature threshold has been crossed. >>>> >>>> The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown >>>> upon hitting over-temperature stage 3 (145 C). Software won't receive an >>>> interrupt in this case because all power is cut. >>> >>> This information is very useful, thanks David! >>> >>> The (partial) hardware shutdown seems like a good measure of last >>> resort, however I suppose we prefer Linux to initiate a shutdown >>> before losing part of the peripherals (drivers might not be happy >>> about this and probably not revover even when the temperature goes >>> down again) or reach a full PMIC shutdown. >>> >>> Please let me know if there are reasons to prefer to go the hardware >>> limits, it's also an option for device makers to overwrite these >>> settings if they want different behavior. >> >> Disabling stage 3 automatic full PMIC shutdown at 145 C is definitely a >> bad idea. This exists as a last resort in order to save the hardware and >> ensure end user safety in case of excessive temperature even if software >> is locked up. >> >> Disabling stage 2 automatic partial PMIC shutdown at 125 C is not >> recommended as the PMIC is already outside of reasonable operating >> conditions and needs to take corrective action quickly. However, doing so >> may be acceptable if software is taking action to shut down the system >> immediately upon receiving the stage 2 over-temperature interrupt. >> Just to confirm: is it expected that at stage 2 the CPU's on the SoC > should continue running even with partial PMIC shutdown enabled? This is not guaranteed. > It sounded to me like partial PMIC shutdown was supposed to shut down > high-power rails that were not essential to the task of performing an > orderly shutdown. Shutting down high-power peripherals is accurate; however, special care is not taken to ensure that an orderly shutdown is possible. At the very least, the HW and SW state will be out of sync for the peripherals that are shut down. > I think Matthias was seeing that when he reached stage 2 and partial > PMIC shutdown happened that the system was just falling on the floor. > ...maybe we just have things configured incorrectly? More information about the exact crash steps would be helpful to investigate this further. I'm not sure how much time you want to put into it though. Disabling stage 2 partial shutdown and then using software to perform a controlled shutdown at 125 C is probably the best option for you at this point. Take care, David -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-07-11 22:36 ` David Collins @ 2018-07-11 22:43 ` Doug Anderson 2018-07-11 22:53 ` Matthias Kaehlcke 2018-07-12 0:10 ` David Collins 0 siblings, 2 replies; 16+ messages in thread From: Doug Anderson @ 2018-07-11 22:43 UTC (permalink / raw) To: David Collins Cc: Matthias Kaehlcke, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd Hi On Wed, Jul 11, 2018 at 3:36 PM, David Collins <collinsd@codeaurora.org> wrote: > Hello Doug, > >> On Tue, Jul 10, 2018 at 10:45 AM, David Collins <collinsd@codeaurora.org> wrote: >>> On 06/29/2018 04:54 PM, Matthias Kaehlcke wrote: >>>> On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: >>> ... >>>>> The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial >>>>> PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns >>>>> off peripherals within the PMIC that are expected to draw significant >>>>> current. The set of peripherals included varies between PMICs. This >>>>> partial shutdown will occur simultaneously with the triggering of an >>>>> interrupt to the APPS processor that informs the qcom-spmi-temp-alarm >>>>> driver that an over-temperature threshold has been crossed. >>>>> >>>>> The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown >>>>> upon hitting over-temperature stage 3 (145 C). Software won't receive an >>>>> interrupt in this case because all power is cut. >>>> >>>> This information is very useful, thanks David! >>>> >>>> The (partial) hardware shutdown seems like a good measure of last >>>> resort, however I suppose we prefer Linux to initiate a shutdown >>>> before losing part of the peripherals (drivers might not be happy >>>> about this and probably not revover even when the temperature goes >>>> down again) or reach a full PMIC shutdown. >>>> >>>> Please let me know if there are reasons to prefer to go the hardware >>>> limits, it's also an option for device makers to overwrite these >>>> settings if they want different behavior. >>> >>> Disabling stage 3 automatic full PMIC shutdown at 145 C is definitely a >>> bad idea. This exists as a last resort in order to save the hardware and >>> ensure end user safety in case of excessive temperature even if software >>> is locked up. >>> >>> Disabling stage 2 automatic partial PMIC shutdown at 125 C is not >>> recommended as the PMIC is already outside of reasonable operating >>> conditions and needs to take corrective action quickly. However, doing so >>> may be acceptable if software is taking action to shut down the system >>> immediately upon receiving the stage 2 over-temperature interrupt. >>> Just to confirm: is it expected that at stage 2 the CPU's on the SoC >> should continue running even with partial PMIC shutdown enabled? > > This is not guaranteed. > > >> It sounded to me like partial PMIC shutdown was supposed to shut down >> high-power rails that were not essential to the task of performing an >> orderly shutdown. > > Shutting down high-power peripherals is accurate; however, special care is > not taken to ensure that an orderly shutdown is possible. At the very > least, the HW and SW state will be out of sync for the peripherals that > are shut down. OK, I guess I'm confused now. Why does partial PMIC shutdown even exist then? What is the point of leaving some rails alive if software could stop running? It seems like it would be better to just shut everything down. Said another way: can you describe what benefit you see for only partially shutting down the PMIC at stage 2 compared to just fully shutting it down at stage 2? >> I think Matthias was seeing that when he reached stage 2 and partial >> PMIC shutdown happened that the system was just falling on the floor. >> ...maybe we just have things configured incorrectly? > > More information about the exact crash steps would be helpful to > investigate this further. I'm not sure how much time you want to put into > it though. Matthias can add more, but basically he heated the system up and when it reached the stage 2 shutdown it was no longer responsive. > Disabling stage 2 partial shutdown and then using software to > perform a controlled shutdown at 125 C is probably the best option for you > at this point. This seems OK to me given that I don't understand the original purpose of the partial PMIC shutdown. Would you expect that all upstream PMIC users would want stage 2 partial shutdown disabled, so we should just do this for all users of the PMIC? -Doug ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-07-11 22:43 ` Doug Anderson @ 2018-07-11 22:53 ` Matthias Kaehlcke 2018-07-12 0:10 ` David Collins 1 sibling, 0 replies; 16+ messages in thread From: Matthias Kaehlcke @ 2018-07-11 22:53 UTC (permalink / raw) To: Doug Anderson Cc: David Collins, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd On Wed, Jul 11, 2018 at 03:43:34PM -0700, Doug Anderson wrote: > Hi > > On Wed, Jul 11, 2018 at 3:36 PM, David Collins <collinsd@codeaurora.org> wrote: > > Hello Doug, > > > >> On Tue, Jul 10, 2018 at 10:45 AM, David Collins <collinsd@codeaurora.org> wrote: > >>> On 06/29/2018 04:54 PM, Matthias Kaehlcke wrote: > >>>> On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: > >>> ... > >>>>> The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial > >>>>> PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns > >>>>> off peripherals within the PMIC that are expected to draw significant > >>>>> current. The set of peripherals included varies between PMICs. This > >>>>> partial shutdown will occur simultaneously with the triggering of an > >>>>> interrupt to the APPS processor that informs the qcom-spmi-temp-alarm > >>>>> driver that an over-temperature threshold has been crossed. > >>>>> > >>>>> The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown > >>>>> upon hitting over-temperature stage 3 (145 C). Software won't receive an > >>>>> interrupt in this case because all power is cut. > >>>> > >>>> This information is very useful, thanks David! > >>>> > >>>> The (partial) hardware shutdown seems like a good measure of last > >>>> resort, however I suppose we prefer Linux to initiate a shutdown > >>>> before losing part of the peripherals (drivers might not be happy > >>>> about this and probably not revover even when the temperature goes > >>>> down again) or reach a full PMIC shutdown. > >>>> > >>>> Please let me know if there are reasons to prefer to go the hardware > >>>> limits, it's also an option for device makers to overwrite these > >>>> settings if they want different behavior. > >>> > >>> Disabling stage 3 automatic full PMIC shutdown at 145 C is definitely a > >>> bad idea. This exists as a last resort in order to save the hardware and > >>> ensure end user safety in case of excessive temperature even if software > >>> is locked up. > >>> > >>> Disabling stage 2 automatic partial PMIC shutdown at 125 C is not > >>> recommended as the PMIC is already outside of reasonable operating > >>> conditions and needs to take corrective action quickly. However, doing so > >>> may be acceptable if software is taking action to shut down the system > >>> immediately upon receiving the stage 2 over-temperature interrupt. > >>> Just to confirm: is it expected that at stage 2 the CPU's on the SoC > >> should continue running even with partial PMIC shutdown enabled? > > > > This is not guaranteed. > > > > > >> It sounded to me like partial PMIC shutdown was supposed to shut down > >> high-power rails that were not essential to the task of performing an > >> orderly shutdown. > > > > Shutting down high-power peripherals is accurate; however, special care is > > not taken to ensure that an orderly shutdown is possible. At the very > > least, the HW and SW state will be out of sync for the peripherals that > > are shut down. > > OK, I guess I'm confused now. Why does partial PMIC shutdown even > exist then? What is the point of leaving some rails alive if software > could stop running? It seems like it would be better to just shut > everything down. > > Said another way: can you describe what benefit you see for only > partially shutting down the PMIC at stage 2 compared to just fully > shutting it down at stage 2? > > > >> I think Matthias was seeing that when he reached stage 2 and partial > >> PMIC shutdown happened that the system was just falling on the floor. > >> ...maybe we just have things configured incorrectly? > > > > More information about the exact crash steps would be helpful to > > investigate this further. I'm not sure how much time you want to put into > > it though. > > Matthias can add more, but basically he heated the system up and when > it reached the stage 2 shutdown it was no longer responsive. The system behaved as on a warm reset when reaching stage 2 temperature, no kernel crash, but messages in /dev/pstore were preserved. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-07-11 22:43 ` Doug Anderson 2018-07-11 22:53 ` Matthias Kaehlcke @ 2018-07-12 0:10 ` David Collins 2018-07-13 16:49 ` Matthias Kaehlcke 1 sibling, 1 reply; 16+ messages in thread From: David Collins @ 2018-07-12 0:10 UTC (permalink / raw) To: Doug Anderson Cc: Matthias Kaehlcke, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd Hello Doug, On 07/11/2018 03:43 PM, Doug Anderson wrote: > On Wed, Jul 11, 2018 at 3:36 PM, David Collins <collinsd@codeaurora.org> wrote: >>> On Tue, Jul 10, 2018 at 10:45 AM, David Collins <collinsd@codeaurora.org> wrote: >>>> On 06/29/2018 04:54 PM, Matthias Kaehlcke wrote: >>>>> On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: >>>> ... >>>>>> The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial >>>>>> PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns >>>>>> off peripherals within the PMIC that are expected to draw significant >>>>>> current. The set of peripherals included varies between PMICs. This >>>>>> partial shutdown will occur simultaneously with the triggering of an >>>>>> interrupt to the APPS processor that informs the qcom-spmi-temp-alarm >>>>>> driver that an over-temperature threshold has been crossed. >>>>>> >>>>>> The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown >>>>>> upon hitting over-temperature stage 3 (145 C). Software won't receive an >>>>>> interrupt in this case because all power is cut. >>>>> >>>>> This information is very useful, thanks David! >>>>> >>>>> The (partial) hardware shutdown seems like a good measure of last >>>>> resort, however I suppose we prefer Linux to initiate a shutdown >>>>> before losing part of the peripherals (drivers might not be happy >>>>> about this and probably not revover even when the temperature goes >>>>> down again) or reach a full PMIC shutdown. >>>>> >>>>> Please let me know if there are reasons to prefer to go the hardware >>>>> limits, it's also an option for device makers to overwrite these >>>>> settings if they want different behavior. >>>> >>>> Disabling stage 3 automatic full PMIC shutdown at 145 C is definitely a >>>> bad idea. This exists as a last resort in order to save the hardware and >>>> ensure end user safety in case of excessive temperature even if software >>>> is locked up. >>>> >>>> Disabling stage 2 automatic partial PMIC shutdown at 125 C is not >>>> recommended as the PMIC is already outside of reasonable operating >>>> conditions and needs to take corrective action quickly. However, doing so >>>> may be acceptable if software is taking action to shut down the system >>>> immediately upon receiving the stage 2 over-temperature interrupt. >>>> Just to confirm: is it expected that at stage 2 the CPU's on the SoC >>> should continue running even with partial PMIC shutdown enabled? >> >> This is not guaranteed. >> >> >>> It sounded to me like partial PMIC shutdown was supposed to shut down >>> high-power rails that were not essential to the task of performing an >>> orderly shutdown. >> >> Shutting down high-power peripherals is accurate; however, special care is >> not taken to ensure that an orderly shutdown is possible. At the very >> least, the HW and SW state will be out of sync for the peripherals that >> are shut down. > > OK, I guess I'm confused now. Why does partial PMIC shutdown even > exist then? What is the point of leaving some rails alive if software > could stop running? It seems like it would be better to just shut > everything down. > > Said another way: can you describe what benefit you see for only > partially shutting down the PMIC at stage 2 compared to just fully > shutting it down at stage 2? Stage 2 partial shutdown is present on PM8998 for legacy reasons. It is being phased out on future PMICs. My understanding is that it was originally intended to be a less aggressive mitigation option than a full shutdown and that it allows for more post-mitigation analysis (e.g. preserved RAM contents). The set of peripherals which are disabled during stage 2 partial shutdown is not well defined which leads to the kind of uncertainty and ill-defined behavior being discussed in this thread. >> Disabling stage 2 partial shutdown and then using software to >> perform a controlled shutdown at 125 C is probably the best option for you >> at this point. > > This seems OK to me given that I don't understand the original purpose > of the partial PMIC shutdown. Would you expect that all upstream PMIC > users would want stage 2 partial shutdown disabled, so we should just > do this for all users of the PMIC? I'd think that we only want to override stage 2 partial shutdown if thermal nodes are defined which cause a graceful software controlled shutdown in place of the PMIC partial shutdown. Therefore, management of the feature should probably be tied to a boolean DT property. Take care, David -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone 2018-07-12 0:10 ` David Collins @ 2018-07-13 16:49 ` Matthias Kaehlcke 0 siblings, 0 replies; 16+ messages in thread From: Matthias Kaehlcke @ 2018-07-13 16:49 UTC (permalink / raw) To: David Collins Cc: Doug Anderson, Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, Stephen Boyd On Wed, Jul 11, 2018 at 05:10:50PM -0700, David Collins wrote: > Hello Doug, > > On 07/11/2018 03:43 PM, Doug Anderson wrote: > > On Wed, Jul 11, 2018 at 3:36 PM, David Collins <collinsd@codeaurora.org> wrote: > >>> On Tue, Jul 10, 2018 at 10:45 AM, David Collins <collinsd@codeaurora.org> wrote: > >>>> On 06/29/2018 04:54 PM, Matthias Kaehlcke wrote: > >>>>> On Fri, Jun 29, 2018 at 02:29:55PM -0700, David Collins wrote: > >>>> ... > >>>>>> The PMIC TEMP_ALARM hardware peripheral will perform an automatic partial > >>>>>> PMIC shutdown upon hitting over-temperature stage 2 (125 C). This turns > >>>>>> off peripherals within the PMIC that are expected to draw significant > >>>>>> current. The set of peripherals included varies between PMICs. This > >>>>>> partial shutdown will occur simultaneously with the triggering of an > >>>>>> interrupt to the APPS processor that informs the qcom-spmi-temp-alarm > >>>>>> driver that an over-temperature threshold has been crossed. > >>>>>> > >>>>>> The TEMP_ALARM peripheral will perform an automatic full PMIC shutdown > >>>>>> upon hitting over-temperature stage 3 (145 C). Software won't receive an > >>>>>> interrupt in this case because all power is cut. > >>>>> > >>>>> This information is very useful, thanks David! > >>>>> > >>>>> The (partial) hardware shutdown seems like a good measure of last > >>>>> resort, however I suppose we prefer Linux to initiate a shutdown > >>>>> before losing part of the peripherals (drivers might not be happy > >>>>> about this and probably not revover even when the temperature goes > >>>>> down again) or reach a full PMIC shutdown. > >>>>> > >>>>> Please let me know if there are reasons to prefer to go the hardware > >>>>> limits, it's also an option for device makers to overwrite these > >>>>> settings if they want different behavior. > >>>> > >>>> Disabling stage 3 automatic full PMIC shutdown at 145 C is definitely a > >>>> bad idea. This exists as a last resort in order to save the hardware and > >>>> ensure end user safety in case of excessive temperature even if software > >>>> is locked up. > >>>> > >>>> Disabling stage 2 automatic partial PMIC shutdown at 125 C is not > >>>> recommended as the PMIC is already outside of reasonable operating > >>>> conditions and needs to take corrective action quickly. However, doing so > >>>> may be acceptable if software is taking action to shut down the system > >>>> immediately upon receiving the stage 2 over-temperature interrupt. > >>>> Just to confirm: is it expected that at stage 2 the CPU's on the SoC > >>> should continue running even with partial PMIC shutdown enabled? > >> > >> This is not guaranteed. > >> > >> > >>> It sounded to me like partial PMIC shutdown was supposed to shut down > >>> high-power rails that were not essential to the task of performing an > >>> orderly shutdown. > >> > >> Shutting down high-power peripherals is accurate; however, special care is > >> not taken to ensure that an orderly shutdown is possible. At the very > >> least, the HW and SW state will be out of sync for the peripherals that > >> are shut down. > > > > OK, I guess I'm confused now. Why does partial PMIC shutdown even > > exist then? What is the point of leaving some rails alive if software > > could stop running? It seems like it would be better to just shut > > everything down. > > > > Said another way: can you describe what benefit you see for only > > partially shutting down the PMIC at stage 2 compared to just fully > > shutting it down at stage 2? > > Stage 2 partial shutdown is present on PM8998 for legacy reasons. It is > being phased out on future PMICs. My understanding is that it was > originally intended to be a less aggressive mitigation option than a full > shutdown and that it allows for more post-mitigation analysis (e.g. > preserved RAM contents). > > The set of peripherals which are disabled during stage 2 partial shutdown > is not well defined which leads to the kind of uncertainty and ill-defined > behavior being discussed in this thread. Thanks for the information! > >> Disabling stage 2 partial shutdown and then using software to > >> perform a controlled shutdown at 125 C is probably the best option for you > >> at this point. > > > > This seems OK to me given that I don't understand the original purpose > > of the partial PMIC shutdown. Would you expect that all upstream PMIC > > users would want stage 2 partial shutdown disabled, so we should just > > do this for all users of the PMIC? > > I'd think that we only want to override stage 2 partial shutdown if > thermal nodes are defined which cause a graceful software controlled > shutdown in place of the PMIC partial shutdown. Therefore, management of > the feature should probably be tied to a boolean DT property. Sounds good, I'll send a patch to disable the partial shutdown through a DT property soon. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node 2018-06-28 21:09 [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Matthias Kaehlcke 2018-06-28 21:09 ` [PATCH 2/3] arm64: dts: qcom: pm8998: Add spmi-temp-alarm node Matthias Kaehlcke 2018-06-28 21:09 ` [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone Matthias Kaehlcke @ 2018-06-28 22:52 ` Doug Anderson 2018-06-29 18:30 ` Matthias Kaehlcke 2 siblings, 1 reply; 16+ messages in thread From: Doug Anderson @ 2018-06-28 22:52 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, David Collins, Stephen Boyd Hi, On Thu, Jun 28, 2018 at 2:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > The node is empty for now. It is needed to allow other DT snippets > to add thermal zone entries. > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > --- > Sorry if you received this twice, lists were missing in cc in the first attempt ... > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 00722b533a92..968390fb0933 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -9,6 +9,7 @@ > #include <dt-bindings/clock/qcom,rpmh.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/soc/qcom,rpmh-rsc.h> > +#include <dt-bindings/thermal/thermal.h> > > / { > interrupt-parent = <&intc>; > @@ -1087,4 +1088,7 @@ > }; > }; > }; > + > + thermal_zones: thermal-zones { > + }; Others can feel free to disagree, but IMO ${SUBJECT} patch is not needed. Instead patch #3 in your series should just define a "thermal-zones" node at the root level. The "thermal-zones" node is a well-defined node with a well-defined node name and location (see the "thermal.txt" binding). If the SoC has a "thermal-zones" node and so does the PMIC then they'll just be merged. -Doug ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node 2018-06-28 22:52 ` [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Doug Anderson @ 2018-06-29 18:30 ` Matthias Kaehlcke 0 siblings, 0 replies; 16+ messages in thread From: Matthias Kaehlcke @ 2018-06-29 18:30 UTC (permalink / raw) To: Doug Anderson Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon, open list:ARM/QUALCOMM SUPPORT, linux-arm-msm, Linux ARM, LKML, David Collins, Stephen Boyd On Thu, Jun 28, 2018 at 03:52:57PM -0700, Doug Anderson wrote: > Hi, > > On Thu, Jun 28, 2018 at 2:09 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > > The node is empty for now. It is needed to allow other DT snippets > > to add thermal zone entries. > > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > --- > > Sorry if you received this twice, lists were missing in cc in the first attempt ... > > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index 00722b533a92..968390fb0933 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -9,6 +9,7 @@ > > #include <dt-bindings/clock/qcom,rpmh.h> > > #include <dt-bindings/interrupt-controller/arm-gic.h> > > #include <dt-bindings/soc/qcom,rpmh-rsc.h> > > +#include <dt-bindings/thermal/thermal.h> > > > > / { > > interrupt-parent = <&intc>; > > @@ -1087,4 +1088,7 @@ > > }; > > }; > > }; > > + > > + thermal_zones: thermal-zones { > > + }; > > Others can feel free to disagree, but IMO ${SUBJECT} patch is not > needed. Instead patch #3 in your series should just define a > "thermal-zones" node at the root level. The "thermal-zones" node is a > well-defined node with a well-defined node name and location (see the > "thermal.txt" binding). If the SoC has a "thermal-zones" node and so > does the PMIC then they'll just be merged. Just defining it here makes perfect sense, thanks! ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-07-13 16:49 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-28 21:09 [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Matthias Kaehlcke 2018-06-28 21:09 ` [PATCH 2/3] arm64: dts: qcom: pm8998: Add spmi-temp-alarm node Matthias Kaehlcke 2018-06-28 21:09 ` [PATCH 3/3] arm64: dts: qcom: pm8998: Add thermal zone Matthias Kaehlcke 2018-06-28 22:58 ` Doug Anderson 2018-06-29 18:51 ` Matthias Kaehlcke 2018-06-29 21:29 ` David Collins 2018-06-29 23:54 ` Matthias Kaehlcke 2018-07-10 17:45 ` David Collins 2018-07-11 21:56 ` Doug Anderson 2018-07-11 22:36 ` David Collins 2018-07-11 22:43 ` Doug Anderson 2018-07-11 22:53 ` Matthias Kaehlcke 2018-07-12 0:10 ` David Collins 2018-07-13 16:49 ` Matthias Kaehlcke 2018-06-28 22:52 ` [PATCH 1/3] arm64: dts: qcom: sdm845: Add thermal-zones node Doug Anderson 2018-06-29 18:30 ` Matthias Kaehlcke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).