arm64: dts: qcom: sc7280: Add PMIC peripherals for SC7280
Subsystems (4)
PATCH 1/1 arm64: dts: qcom: sc7280: Add PMIC peripherals for SC7280
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403
arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsi
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +60 -0
@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pm7325: pmic@1 { + compatible = "qcom,pm7325", "qcom,spmi-pmic"; + reg = <0x1 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pm7325_tz: temp-alarm@a00 { + compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pm7325_gpios: gpios@8800 { + compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller; + gpio-ranges = <&pm7325_gpios 0 0 10>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; +}; + +&thermal_zones { + pm7325_temp_alarm: pm7325_tz { + polling-delay-passive = <100>; + polling-delay = <0>; + thermal-governor = "step_wise"; + thermal-sensors = <&pm7325_tz>; + + trips { + pm7325_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pm7325_trip1: trip1 { + temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pm7325_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + }; + }; + }; +};
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +60 -0
60 lines not on this page · raw message
arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +104 -0
104 lines not on this page · raw message
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +60 -0
60 lines not on this page · raw message
arch/arm64/boot/dts/qcom/sc7280.dtsi +8 -0
@@ -606,4 +606,12 @@ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; }; + + thermal_zones: thermal-zones { + }; }; + +#include "pm7325.dtsi" +#include "pm8350c.dtsi" +#include "pmk8350.dtsi" +#include "pmr735a.dtsi"
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Quoted 3 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Overall this looks good, just two small things below.
Quoted 46 lines
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsiarch/arm64/boot/dts/qcom/pm7325.dtsi new file +24
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pm7325: pmic@1 { + compatible = "qcom,pm7325", "qcom,spmi-pmic"; + reg = <0x1 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pm7325_tz: temp-alarm@a00 { + compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pm7325_gpios: gpios@8800 { + compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller;
arch/arm64/boot/dts/qcom/pm7325.dtsi line 34
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ gpio-ranges = <&pm7325_gpios 0 0 10>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; +}; + +&thermal_zones { + pm7325_temp_alarm: pm7325_tz {
'_' is not allowed to be used in node names, there's a few of these sprinkled through the patch. Please replace them with '-'.
Quoted 26 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +26
-0+ polling-delay-passive = <100>; + polling-delay = <0>; + thermal-governor = "step_wise"; + thermal-sensors = <&pm7325_tz>; + + trips { + pm7325_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pm7325_trip1: trip1 { + temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pm7325_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + }; + }; + }; +};
[..]
Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +0
-0@@ -606,4 +606,12 @@ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
arch/arm64/boot/dts/qcom/sc7280.dtsi line 617
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +8
-0}; + + thermal_zones: thermal-zones { + }; }; + +#include "pm7325.dtsi" +#include "pm8350c.dtsi" +#include "pmk8350.dtsi" +#include "pmr735a.dtsi"
Is there any particular reason for you including these at the end of sc7270.dtsi, rather than the top like we do in other platforms?
Also, are all SC7280 devices always coming with this quartet? We've seen variations of this in the past and therefor typically include them from the board dts instead.
Regards,
Bjorn
Hi Bjorn,
Quoted 60 lines · Bjorn Andersson
Quoted 3 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Overall this looks good, just two small things below.
Quoted 52 lines
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsiarch/arm64/boot/dts/qcom/pm7325.dtsi new file +29
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pm7325: pmic@1 { + compatible = "qcom,pm7325", "qcom,spmi-pmic"; + reg = <0x1 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pm7325_tz: temp-alarm@a00 { + compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pm7325_gpios: gpios@8800 { + compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller; + gpio-ranges = <&pm7325_gpios 0 0 10>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + };
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi line 34
Quoted 5 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +5
-0+ }; +}; + +&thermal_zones { + pm7325_temp_alarm: pm7325_tz {'_' is not allowed to be used in node names, there's a few of these sprinkled through the patch. Please replace them with '-'.
Okay, will replace them.
Quoted 48 lines
Quoted 26 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +26
-0+ polling-delay-passive = <100>; + polling-delay = <0>; + thermal-governor = "step_wise"; + thermal-sensors = <&pm7325_tz>; + + trips { + pm7325_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pm7325_trip1: trip1 { + temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pm7325_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + }; + }; + }; +};[..]
Quoted 7 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +0
-0@@ -606,4 +606,12 @@ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;arch/arm64/boot/dts/qcom/sc7280.dtsi line 617
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +8
-0}; + + thermal_zones: thermal-zones { + }; }; + +#include "pm7325.dtsi" +#include "pm8350c.dtsi" +#include "pmk8350.dtsi" +#include "pmr735a.dtsi"Is there any particular reason for you including these at the end of
sc7270.dtsi, rather than the top like we do in other platforms?
Quoted 4 lines
Also, are all SC7280 devices always coming with this quartet? We've seen variations of this in the past and therefor typically include them from the board dts instead.
No specific reason, will add them in board dts file.
Quoted 2 lines
Regards,
Bjorn
Thanks,
Satya Priya
Hi Satya,
Quoted 29 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsiarch/arm64/boot/dts/qcom/pm7325.dtsi new file +4
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h>
arch/arm64/boot/dts/qcom/pm7325.dtsi line 14
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pm7325: pmic@1 { + compatible = "qcom,pm7325", "qcom,spmi-pmic"; + reg = <0x1 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pm7325_tz: temp-alarm@a00 {
The _tz suffix suggests this is a thermal zone, which isn't the case. Call it 'pm7325_temp_alarm' or similar.
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pm7325_gpios: gpios@8800 { + compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller;
arch/arm64/boot/dts/qcom/pm7325.dtsi line 34
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ gpio-ranges = <&pm7325_gpios 0 0 10>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; +}; + +&thermal_zones { + pm7325_temp_alarm: pm7325_tz {
The temperature corresponds to the PM7325 on-die temperature, the temp alarm is a feature to monitor it. Also many QCA SoCs name the thermal zones <whatever>-thermal, so it seems good to follow this convention. My suggestion is:
pm7325_thermal: pm7325-thermal {arch/arm64/boot/dts/qcom/pm7325.dtsi line 36
Quoted 2 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +2
-0+ polling-delay-passive = <100>; + polling-delay = <0>;
Are you sure that no polling delay is needed? How does the thermal framework detect that the temperatures is >= the passive trip point and that it should start polling at 'polling-delay-passive' rate?
arch/arm64/boot/dts/qcom/pm7325.dtsi line 37
Quoted 1 line
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +1
-0+ thermal-governor = "step_wise";
This property is not supported upstream.
In any case, this thermal zone doesn't have cooling devices, what is any thermal governor supposed to do with this thermal zone?
I understand that the zone is generally useful to configure the over-temperature protection of the PMIC and to allow the kernel to shut down (or reboot) when a critical trip point is reached, but the specific governor is irrelevant as far as I understand.
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ thermal-sensors = <&pm7325_tz>; + + trips { + pm7325_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pm7325_trip1: trip1 {
arch/arm64/boot/dts/qcom/pm7325.dtsi line 57
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pm7325_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + };
Why are there two critical trip points? The system should shut down when the first trip point is reached, the second one is irrelevant. As far as I recall from implementing f1599f9e4cd6 ("thermal:
qcom-spmi: Use PMIC thermal stage 2 for critical trip points") earlier PMIC versions have 3 stages for the over-temperature protection. When the stage 3 threshold (trip2) is hit the PMIC performs and automated shutdown. Unless this has changed for the PM7325 the second critical trip point should not be needed. The second critical trip point could even lead to a misconfiguration of the PMIC threshold, since the driver interprets the temperature of a critical trip point as the stage3 temperature.
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +3
-0+ }; + }; +};arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +0
-0
arch/arm64/boot/dts/qcom/pm8350c.dtsi line 8
Quoted 8 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +8
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pm8350: pmic@2 {
pm8350c ?
arch/arm64/boot/dts/qcom/pm8350c.dtsi line 14
Quoted 6 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +6
-0+ compatible = "qcom,pm8350c", "qcom,spmi-pmic"; + reg = <0x2 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pm8350c_tz: temp-alarm@a00 {
same as for pm7325
Quoted 14 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +14
-0+ compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pm8350c_gpios: gpios@8800 { + compatible = "qcom,pm8350c-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller; + gpio-ranges = <&pm8350c_gpios 0 0 9>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>;
arch/arm64/boot/dts/qcom/pm8350c.dtsi line 38
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +10
-0+ }; + }; +}; + +&thermal_zones { + pm8350c_temp_alarm: pm8350c_tz { + polling-delay-passive = <100>; + polling-delay = <0>; + thermal-governor = "step_wise"; + thermal-sensors = <&pm8350c_tz>;
same as for pm7325
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +9
-0+ + trips { + pm8350c_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pm8350c_trip1: trip1 {
arch/arm64/boot/dts/qcom/pm8350c.dtsi line 57
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +10
-0+ temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pm8350c_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + };
same as for pm7325
Quoted 19 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +3
-0+ }; + }; +};arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +10
-0@@ -0,0 +1,104 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/input/input.h> +#include <dt-bindings/input/linux-event-codes.h> +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> +#include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h> +#include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h> +#include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>
arch/arm64/boot/dts/qcom/pmk8350.dtsi line 20
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +10
-0+#include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h> + +&spmi_bus { + pmk8350: pmic@0 { + compatible = "qcom,pmk8350", "qcom,spmi-pmic"; + reg = <0x0 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pmk8350_pon: pon_hlos@1300 {
just pon@1300 for consistency? That's also the namingMK8350 Channel nodes */
arch/arm64/boot/dts/qcom/pmk8350.dtsi line 54
Quoted 7 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +7
-0+ pmk8350_die_temp { + reg = <PMK8350_ADC7_DIE_TEMP>; + label = "pmk8350_die_temp"; + qcom,pre-scaling = <1 1>; + }; + + /* PM8350 Channel nodes */
This comment and the ones below don't seem particularly useful, the naming of the nodes should provide enough context.
Quoted 60 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +50
-0+ pm8350_die_temp { + reg = <PM8350_ADC7_DIE_TEMP>; + label = "pm8350_die_temp"; + qcom,pre-scaling = <1 1>; + }; + + /* PMR735a Channel nodes */ + pmr735a_die_temp { + reg = <PMR735A_ADC7_DIE_TEMP>; + label = "pmr735a_die_temp"; + qcom,pre-scaling = <1 1>; + }; + + /* PMR735b Channel nodes */ + pmr735b_die_temp { + reg = <PMR735B_ADC7_DIE_TEMP>; + label = "pmr735b_die_temp"; + qcom,pre-scaling = <1 1>; + }; + }; + + pmk8350_adc_tm: adc_tm@3400 { + compatible = "qcom,adc-tm7"; + reg = <0x3400>; + interrupts = <0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "threshold"; + #address-cells = <1>; + #size-cells = <0>; + #thermal-sensor-cells = <1>; + status = "disabled"; + }; + + pmk8350_gpios: gpios@b000 { + compatible = "qcom,pmk8350-gpio", "qcom,spmi-gpio"; + reg = <0xb000>; + gpio-controller; + gpio-ranges = <&pmk8350_gpios 0 0 4>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + + pmk8350_rtc: rtc@6100 { + compatible = "qcom,pmk8350-rtc"; + reg = <0x6100>, <0x6200>; + reg-names = "rtc", "alarm"; + interrupts = <0x0 0x62 0x1 IRQ_TYPE_EDGE_RISING>; + }; + }; +};arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +4
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h>
arch/arm64/boot/dts/qcom/pmr735a.dtsi line 14
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +10
-0+#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pmr735a: pmic@4 { + compatible = "qcom,pmr735a", "qcom,spmi-pmic"; + reg = <0x4 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pmr735a_tz: temp-alarm@a00 {
same as for pm7325
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +10
-0+ compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x4 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pmr735a_gpios: gpios@8800 { + compatible = "qcom,pmr735a-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller;
arch/arm64/boot/dts/qcom/pmr735a.dtsi line 34
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +10
-0+ gpio-ranges = <&pmr735a_gpios 0 0 4>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; +}; + +&thermal_zones { + pmr735a_temp_alarm: pmr735a_tz {
same as for pm7325
arch/arm64/boot/dts/qcom/pmr735a.dtsi line 38
Quoted 4 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +4
-0+ polling-delay-passive = <100>; + polling-delay = <0>; + thermal-governor = "step_wise"; + thermal-sensors = <&pmr735a_tz>;
same as for pm7325
Quoted 9 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +9
-0+ + trips { + pmr735a_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pmr735a_trip1: trip1 {
arch/arm64/boot/dts/qcom/pmr735a.dtsi line 57
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +10
-0+ temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pmr735a_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + };
same as for pm7325
Quoted 20 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +3
-0+ }; + }; +};arch/arm64/boot/dts/qcom/sc7280.dtsi +8
-0@@ -606,4 +606,12 @@ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; }; + + thermal_zones: thermal-zones { + }; }; + +#include "pm7325.dtsi" +#include "pm8350c.dtsi" +#include "pmk8350.dtsi" +#include "pmr735a.dtsi"
Hi Matthias,
Quoted 38 lines · Matthias Kaehlcke
Hi Satya,
Quoted 35 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsiarch/arm64/boot/dts/qcom/pm7325.dtsi new file +9
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pm7325: pmic@1 { + compatible = "qcom,pm7325", "qcom,spmi-pmic";
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi line 14
Quoted 5 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +5
-0+ reg = <0x1 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pm7325_tz: temp-alarm@a00 {The
_tzsuffix suggests this is a thermal zone, which isn't the case. Call it 'pm7325_temp_alarm' or similar.
Okay.
Quoted 25 lines
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pm7325_gpios: gpios@8800 { + compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller;arch/arm64/boot/dts/qcom/pm7325.dtsi line 34
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ gpio-ranges = <&pm7325_gpios 0 0 10>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; +}; + +&thermal_zones { + pm7325_temp_alarm: pm7325_tz {The temperature corresponds to the PM7325 on-die temperature, the temp alarm is a feature to monitor it. Also many QCA SoCs name the thermal zones <whatever>-thermal, so it seems good to follow this convention. My suggestion is:
Quoted 2 lines
pm7325_thermal: pm7325-thermal {Okay, will change it.
Quoted 8 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi line 36
Quoted 3 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +2
-0+ polling-delay-passive = <100>; + polling-delay = <0>;Are you sure that no polling delay is needed? How does the thermal framework detect that the temperatures is >= the passive trip point and that it should start polling at 'polling-delay-passive' rate?
As the temp-alarm has interrupt support, whenever preconfigured threshold violates it notifies thermal framework, so I think the polling delay is not needed here.
Quoted 4 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi line 37
Quoted 1 line
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +1
-0+ thermal-governor = "step_wise";This property is not supported upstream.
Quoted 8 lines
In any case, this thermal zone doesn't have cooling devices, what is any thermal governor supposed to do with this thermal zone?
I understand that the zone is generally useful to configure the over-temperature protection of the PMIC and to allow the kernel to shut down (or reboot) when a critical trip point is reached, but the specific governor is irrelevant as far as I understand.
Yeah, this is not required here will remove it.
Quoted 24 lines
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ thermal-sensors = <&pm7325_tz>; + + trips { + pm7325_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pm7325_trip1: trip1 {arch/arm64/boot/dts/qcom/pm7325.dtsi line 57
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +10
-0+ temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pm7325_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + };Why are there two critical trip points? The system should shut down when the first trip point is reached, the second one is irrelevant. As far as I recall from implementing
f1599f9e4cd6("thermal:
Quoted 9 lines
qcom-spmi: Use PMIC thermal stage 2 for critical trip points") earlier PMIC versions have 3 stages for the over-temperature protection. When the stage 3 threshold (trip2) is hit the PMIC performs and automated shutdown. Unless this has changed for the PM7325 the second critical trip point should not be needed. The second critical trip point could even lead to a misconfiguration of the PMIC threshold, since the driver interprets the temperature of a critical trip point as the stage3 temperature.
yes I agree, the system will shutdown when we hit the 1st critical, 2nd critical point is not needed. Probably this got carried from old change propagation, will remove it.
Quoted 11 lines
Quoted 11 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +3
-0+ }; + }; +};arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +2
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved.
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi line 8
Quoted 6 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +6
-0+ +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pm8350: pmic@2 {pm8350c ?
Ok.
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi line 14
Quoted 6 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +6
-0+ compatible = "qcom,pm8350c", "qcom,spmi-pmic"; + reg = <0x2 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pm8350c_tz: temp-alarm@a00 {same as for pm7325
Ok.
Quoted 73 lines
Quoted 14 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +14
-0+ compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pm8350c_gpios: gpios@8800 { + compatible = "qcom,pm8350c-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller; + gpio-ranges = <&pm8350c_gpios 0 0 9>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>;arch/arm64/boot/dts/qcom/pm8350c.dtsi line 38
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +10
-0+ }; + }; +}; + +&thermal_zones { + pm8350c_temp_alarm: pm8350c_tz { + polling-delay-passive = <100>; + polling-delay = <0>; + thermal-governor = "step_wise"; + thermal-sensors = <&pm8350c_tz>;same as for pm7325
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +9
-0+ + trips { + pm8350c_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pm8350c_trip1: trip1 {arch/arm64/boot/dts/qcom/pm8350c.dtsi line 57
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +10
-0+ temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pm8350c_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + };same as for pm7325
Quoted 23 lines
arch/arm64/boot/dts/qcom/pm8350c.dtsi new file +3
-0+ }; + }; +};arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +14
-0@@ -0,0 +1,104 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/input/input.h> +#include <dt-bindings/input/linux-event-codes.h> +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> +#include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h> +#include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h> +#include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h> +#include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h> + +&spmi_bus { + pmk8350: pmic@0 {
Quoted 8 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi line 20
Quoted 6 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +6
-0+ compatible = "qcom,pmk8350", "qcom,spmi-pmic"; + reg = <0x0 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pmk8350_pon: pon_hlos@1300 {just pon@1300 for consistency? That's also the namingMK8350 Channel nodes */
ok.
Quoted 11 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi line 54
Quoted 7 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +7
-0+ pmk8350_die_temp { + reg = <PMK8350_ADC7_DIE_TEMP>; + label = "pmk8350_die_temp"; + qcom,pre-scaling = <1 1>; + }; + + /* PM8350 Channel nodes */This comment and the ones below don't seem particularly useful, the naming of the nodes should provide enough context.
Ok, will remove.
Quoted 64 lines
Quoted 64 lines
arch/arm64/boot/dts/qcom/pmk8350.dtsi new file +50
-0+ pm8350_die_temp { + reg = <PM8350_ADC7_DIE_TEMP>; + label = "pm8350_die_temp"; + qcom,pre-scaling = <1 1>; + }; + + /* PMR735a Channel nodes */ + pmr735a_die_temp { + reg = <PMR735A_ADC7_DIE_TEMP>; + label = "pmr735a_die_temp"; + qcom,pre-scaling = <1 1>; + }; + + /* PMR735b Channel nodes */ + pmr735b_die_temp { + reg = <PMR735B_ADC7_DIE_TEMP>; + label = "pmr735b_die_temp"; + qcom,pre-scaling = <1 1>; + }; + }; + + pmk8350_adc_tm: adc_tm@3400 { + compatible = "qcom,adc-tm7"; + reg = <0x3400>; + interrupts = <0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "threshold"; + #address-cells = <1>; + #size-cells = <0>; + #thermal-sensor-cells = <1>; + status = "disabled"; + }; + + pmk8350_gpios: gpios@b000 { + compatible = "qcom,pmk8350-gpio", "qcom,spmi-gpio"; + reg = <0xb000>; + gpio-controller; + gpio-ranges = <&pmk8350_gpios 0 0 4>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + + pmk8350_rtc: rtc@6100 { + compatible = "qcom,pmk8350-rtc"; + reg = <0x6100>, <0x6200>; + reg-names = "rtc", "alarm"; + interrupts = <0x0 0x62 0x1 IRQ_TYPE_EDGE_RISING>; + }; + }; +};arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +8
-0@@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/spmi/spmi.h> + +&spmi_bus { + pmr735a: pmic@4 {
Quoted 9 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi line 14
Quoted 6 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +6
-0+ compatible = "qcom,pmr735a", "qcom,spmi-pmic"; + reg = <0x4 SPMI_USID>; + #address-cells = <1>; + #size-cells = <0>; + + pmr735a_tz: temp-alarm@a00 {same as for pm7325
Ok.
Quoted 64 lines
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +10
-0+ compatible = "qcom,spmi-temp-alarm"; + reg = <0xa00>; + interrupts = <0x4 0xa 0x0 IRQ_TYPE_EDGE_BOTH>; + #thermal-sensor-cells = <0>; + }; + + pmr735a_gpios: gpios@8800 { + compatible = "qcom,pmr735a-gpio", "qcom,spmi-gpio"; + reg = <0x8800>; + gpio-controller;arch/arm64/boot/dts/qcom/pmr735a.dtsi line 34
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +10
-0+ gpio-ranges = <&pmr735a_gpios 0 0 4>; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; +}; + +&thermal_zones { + pmr735a_temp_alarm: pmr735a_tz {same as for pm7325
arch/arm64/boot/dts/qcom/pmr735a.dtsi line 38
Quoted 4 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +4
-0+ polling-delay-passive = <100>; + polling-delay = <0>; + thermal-governor = "step_wise"; + thermal-sensors = <&pmr735a_tz>;same as for pm7325
Quoted 9 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +9
-0+ + trips { + pmr735a_trip0: trip0 { + temperature = <95000>; + hysteresis = <0>; + type = "passive"; + }; + + pmr735a_trip1: trip1 {arch/arm64/boot/dts/qcom/pmr735a.dtsi line 57
Quoted 10 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +10
-0+ temperature = <115000>; + hysteresis = <0>; + type = "critical"; + }; + + pmr735a_trip2: trip2 { + temperature = <145000>; + hysteresis = <0>; + type = "critical"; + };same as for pm7325
Quoted 12 lines
arch/arm64/boot/dts/qcom/pmr735a.dtsi new file +3
-0+ }; + }; +};arch/arm64/boot/dts/qcom/sc7280.dtsi +1
-0@@ -606,4 +606,12 @@ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; }; +
Quoted 8 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi line 617
Quoted 8 lines
arch/arm64/boot/dts/qcom/sc7280.dtsi +7
-0+ thermal_zones: thermal-zones { + }; }; + +#include "pm7325.dtsi" +#include "pm8350c.dtsi" +#include "pmk8350.dtsi" +#include "pmr735a.dtsi"
Thanks,
Satya Priya
Hi Satya,
Quoted 47 lines · On Mon, Mar 22, 2021 at 06:50:47PM +0530, [email protected] wrote:
Hi Matthias,
Quoted 34 lines · Matthias Kaehlcke
Hi Satya,
Quoted 28 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsiarch/arm64/boot/dts/qcom/pm7325.dtsi new file +0
-0@@ -0,0 +1,60 @@...
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi
Quoted 2 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +2
-0+ polling-delay-passive = <100>; + polling-delay = <0>;Are you sure that no polling delay is needed? How does the thermal framework
detect that the temperatures is >= the passive trip point and that it should
start polling at 'polling-delay-passive' rate?
Quoted 3 lines
As the temp-alarm has interrupt support, whenever preconfigured threshold violates it notifies thermal framework, so I think the polling delay is not needed here.
From the documentation I found it's not clear to me how exactly these interrupts work. Is a single interrupt triggered when the threshold is violated or are there periodic (?) interrupts as long as the temperature is above the stage 0 threshold?
Why is 'polling-delay-passive' passive needed if there are interrupts? Maybe to detect that the zone should transition from passive to no cooling when the temperature drops below the stage 0 threshold?
Hi Matthias,
Quoted 54 lines · Matthias Kaehlcke
Hi Satya,
Quoted 47 lines · On Mon, Mar 22, 2021 at 06:50:47PM +0530, [email protected] wrote:
Hi Matthias,
Quoted 34 lines · Matthias Kaehlcke
Hi Satya,
Quoted 28 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsiarch/arm64/boot/dts/qcom/pm7325.dtsi new file +0
-0@@ -0,0 +1,60 @@...
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi
Quoted 2 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +2
-0+ polling-delay-passive = <100>; + polling-delay = <0>;Are you sure that no polling delay is needed? How does the thermal framework
detect that the temperatures is >= the passive trip point and that it should
start polling at 'polling-delay-passive' rate?Quoted 3 lines
As the temp-alarm has interrupt support, whenever preconfigured threshold violates it notifies thermal framework, so I think the polling delay is not needed here.
Quoted 8 lines
From the documentation I found it's not clear to me how exactly these interrupts work. Is a single interrupt triggered when the threshold is violated or are there periodic (?) interrupts as long as the temperature is above the stage 0 threshold?
Why is 'polling-delay-passive' passive needed if there are interrupts? Maybe to detect that the zone should transition from passive to no cooling when the temperature drops below the stage 0 threshold?
The PMIC TEMP_ALARM peripheral maintains an internal over-temperature stage: 0, 1, 2, or 3. Stage 0 is normal operation below the lowest (stage 1) threshold [usually 95 C]. When in stage 1, the temperature is between the stage 1 and 2 thresholds [stage 2 threshold is usually 115 C]. Upon hitting the stage 3 threshold [usually 145 C], the PMIC hardware will automatically shut down the system.
The TEMP_ALARM IRQ fires on stage 0 -> 1 and 1 -> 0 transitions. We therefore set polling-delay = <0> since there is no need for software to monitor the temperature periodically when operating in stage 0. Upon crossing the stage 1 threshold, SW receives the IRQ and the thermal framework hits its first trip changing the thermal zone to passive mode.
This then engages the 100 ms polling enabled via polling-delay-passive
= <100>. If the temperate keeps climbing and passes the stage 2 threshold, the thermal framework hits the second trip (which is critical) and it initiates an orderly shutdown. If the temperature drops below the stage 1 threshold, then the thermal framework exits passive mode and stops polling. This approach reduces/eliminates the software overhead when not at an elevated temperature.
Thanks,
Satya Priya
Quoted 77 lines · On Thu, Mar 25, 2021 at 10:50:57AM +0530, [email protected] wrote:
Hi Matthias,
Quoted 61 lines · Matthias Kaehlcke
Hi Satya,
Quoted 47 lines · On Mon, Mar 22, 2021 at 06:50:47PM +0530, [email protected] wrote:
Hi Matthias,
Quoted 34 lines · Matthias Kaehlcke
Hi Satya,
Quoted 28 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsiarch/arm64/boot/dts/qcom/pm7325.dtsi new file +0
-0@@ -0,0 +1,60 @@...
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi
Quoted 2 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi new file +2
-0+ polling-delay-passive = <100>; + polling-delay = <0>;Are you sure that no polling delay is needed? How does the thermal framework
detect that the temperatures is >= the passive trip point and that it should
start polling at 'polling-delay-passive' rate?Quoted 5 lines
As the temp-alarm has interrupt support, whenever preconfigured threshold
violates it notifies thermal framework, so I think the polling delay is not
needed here.From the documentation I found it's not clear to me how exactly these interrupts work. Is a single interrupt triggered when the threshold is violated or are there periodic (?) interrupts as long as the temperature is above the stage 0 threshold?
Quoted 5 lines
Why is 'polling-delay-passive' passive needed if there are interrupts? Maybe
to detect that the zone should transition from passive to no cooling when the
temperature drops below the stage 0 threshold?The PMIC
TEMP_ALARMperipheral maintains an internal over-temperature stage:
0, 1, 2, or 3. Stage 0 is normal operation below the lowest (stage 1) threshold [usually 95 C]. When in stage 1, the temperature is between the stage 1 and 2 thresholds [stage 2 threshold is usually 115 C]. Upon hitting the stage 3 threshold [usually 145 C], the PMIC hardware will automatically shut down the system.
Quoted 12 lines
The
TEMP_ALARMIRQ fires on stage 0 -> 1 and 1 -> 0 transitions. We therefore set polling-delay = <0> since there is no need for software to monitor the temperature periodically when operating in stage 0. Upon crossing the stage 1 threshold, SW receives the IRQ and the thermal framework hits its first trip changing the thermal zone to passive mode. This then engages the 100 ms polling enabled via polling-delay-passive = <100>. If the temperate keeps climbing and passes the stage 2 threshold, the thermal framework hits the second trip (which is critical) and it initiates an orderly shutdown. If the temperature drops below the stage 1 threshold, then the thermal framework exits passive mode and stops polling. This approach reduces/eliminates the software overhead when not at an elevated temperature.
Thanks for the clarification. With the interrupt only firing on stage 0 -> 1 and stage 1 -> 0 it makes sense. I was expecting interrupts on the other transitions too.
Quoted 8 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsi
The subject 'arm64: dts: qcom: sc7280: Add PMIC peripherals for SC7280' is a bit misleading, at least for the git history it would be clearer to to split this into per-PMIC patches and one SC7280 patch.
Quoted 21 lines · Matthias Kaehlcke
Quoted 8 lines · satya priya
Add PM7325/PM8350C/PMK8350/PMR735A peripherals such as PON, GPIOs, RTC and other PMIC infra modules for SC7280.
Signed-off-by satya priya <[email protected]>---
This patch depends on base DT and board files for SC7280 to merge first
https://lore.kernel.org/patchwork/project/lkml/list/?series=487403Quoted 11 lines
arch/arm64/boot/dts/qcom/pm7325.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pm8350c.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmk8350.dtsi | 104 ++++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/pmr735a.dtsi | 60 ++++++++++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 8 +++ 5 files changed, 292 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsi
Quoted 3 lines
The subject 'arm64: dts: qcom: sc7280: Add PMIC peripherals for SC7280' is a bit misleading, at least for the git history it would be clearer to to split this into per-PMIC patches and one SC7280 patch.
Okay, will split them.