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
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
diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi b/arch/arm64/boot/dts/qcom/pm7325.dtsi
new file mode 100644
index 0000000..393b256
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
@@ -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";
+ };
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
new file mode 100644
index 0000000..dffa79d
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -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 {
+ compatible = "qcom,pm8350c", "qcom,spmi-pmic";
+ reg = <0x2 SPMI_USID>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pm8350c_tz: temp-alarm@a00 {
+ 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>;
+ };
+ };
+};
+
+&thermal_zones {
+ pm8350c_temp_alarm: pm8350c_tz {
+ polling-delay-passive = <100>;
+ polling-delay = <0>;
+ thermal-governor = "step_wise";
+ thermal-sensors = <&pm8350c_tz>;
+
+ trips {
+ pm8350c_trip0: trip0 {
+ temperature = <95000>;
+ hysteresis = <0>;
+ type = "passive";
+ };
+
+ pm8350c_trip1: trip1 {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+
+ pm8350c_trip2: trip2 {
+ temperature = <145000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
new file mode 100644
index 0000000..9749484
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
@@ -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 {
+ compatible = "qcom,pmk8350", "qcom,spmi-pmic";
+ reg = <0x0 SPMI_USID>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmk8350_pon: pon_hlos@1300 {
+ compatible = "qcom,pm8998-pon";
+ reg = <0x1300>;
+
+ pwrkey {
+ compatible = "qcom,pmk8350-pwrkey";
+ interrupts = <0x0 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
+ linux,code = <KEY_POWER>;
+ };
+
+ resin {
+ compatible = "qcom,pmk8350-resin";
+ interrupts = <0x0 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
+ linux,code = <KEY_VOLUMEDOWN>;
+ };
+ };
+
+ pmk8350_vadc: adc@3100 {
+ compatible = "qcom,spmi-adc7";
+ reg = <0x3100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "eoc-int-en-set";
+ #io-channel-cells = <1>;
+ io-channel-ranges;
+
+ /* PMK8350 Channel nodes */
+ pmk8350_die_temp {
+ reg = <PMK8350_ADC7_DIE_TEMP>;
+ label = "pmk8350_die_temp";
+ qcom,pre-scaling = <1 1>;
+ };
+
+ /* PM8350 Channel nodes */
+ 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>;
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/pmr735a.dtsi b/arch/arm64/boot/dts/qcom/pmr735a.dtsi
new file mode 100644
index 0000000..e1d2356
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pmr735a.dtsi
@@ -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 {
+ compatible = "qcom,pmr735a", "qcom,spmi-pmic";
+ reg = <0x4 SPMI_USID>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmr735a_tz: temp-alarm@a00 {
+ 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;
+ gpio-ranges = <&pmr735a_gpios 0 0 4>;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+ };
+};
+
+&thermal_zones {
+ pmr735a_temp_alarm: pmr735a_tz {
+ polling-delay-passive = <100>;
+ polling-delay = <0>;
+ thermal-governor = "step_wise";
+ thermal-sensors = <&pmr735a_tz>;
+
+ trips {
+ pmr735a_trip0: trip0 {
+ temperature = <95000>;
+ hysteresis = <0>;
+ type = "passive";
+ };
+
+ pmr735a_trip1: trip1 {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+
+ pmr735a_trip2: trip2 {
+ temperature = <145000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 8af6d77..25402d4 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -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"
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
On Thu 11 Mar 04:40 CST 2021, satya priya wrote:
> 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.
> 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
>
> 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
>
> diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> new file mode 100644
> index 0000000..393b256
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> @@ -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 {
'_' is not allowed to be used in node names, there's a few of these
sprinkled through the patch. Please replace them with '-'.
> + 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";
> + };
> + };
> + };
> +};
[..]
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 8af6d77..25402d4 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -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"
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 Satya,
On Thu, Mar 11, 2021 at 04:10:29PM +0530, satya priya wrote:
> 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
>
> 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
>
> diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> new file mode 100644
> index 0000000..393b256
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> @@ -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 {
The _tz suffix suggests this is a thermal zone, which isn't the case.
Call it 'pm7325_temp_alarm' or similar.
> + 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 {
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 {
> + 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?
> + 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.
> + 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";
> + };
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.
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> new file mode 100644
> index 0000000..dffa79d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> @@ -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 ?
> + 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
> + 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>;
> + };
> + };
> +};
> +
> +&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
> +
> + trips {
> + pm8350c_trip0: trip0 {
> + temperature = <95000>;
> + hysteresis = <0>;
> + type = "passive";
> + };
> +
> + pm8350c_trip1: trip1 {
> + temperature = <115000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
> +
> + pm8350c_trip2: trip2 {
> + temperature = <145000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
same as for pm7325
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> new file mode 100644
> index 0000000..9749484
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> @@ -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 {
> + 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 */
> + 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.
> + 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>;
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/pmr735a.dtsi b/arch/arm64/boot/dts/qcom/pmr735a.dtsi
> new file mode 100644
> index 0000000..e1d2356
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pmr735a.dtsi
> @@ -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 {
> + 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
> + 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;
> + 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
> + polling-delay-passive = <100>;
> + polling-delay = <0>;
> + thermal-governor = "step_wise";
> + thermal-sensors = <&pmr735a_tz>;
same as for pm7325
> +
> + trips {
> + pmr735a_trip0: trip0 {
> + temperature = <95000>;
> + hysteresis = <0>;
> + type = "passive";
> + };
> +
> + pmr735a_trip1: trip1 {
> + temperature = <115000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
> +
> + pmr735a_trip2: trip2 {
> + temperature = <145000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
same as for pm7325
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 8af6d77..25402d4 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -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"
On Thu, Mar 11, 2021 at 04:10:29PM +0530, satya priya wrote:
> 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
>
> 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.
Hi Bjorn,
On 2021-03-12 23:06, Bjorn Andersson wrote:
> On Thu 11 Mar 04:40 CST 2021, satya priya wrote:
>
>> 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.
>
>> 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
>>
>> 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
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> new file mode 100644
>> index 0000000..393b256
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> @@ -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 {
>
> '_' 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.
>> + 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";
>> + };
>> + };
>> + };
>> +};
> [..]
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 8af6d77..25402d4 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -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"
>
> 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.
>
No specific reason, will add them in board dts file.
> Regards,
> Bjorn
Thanks,
Satya Priya
Hi Matthias,
On 2021-03-13 02:10, Matthias Kaehlcke wrote:
> Hi Satya,
>
> On Thu, Mar 11, 2021 at 04:10:29PM +0530, satya priya wrote:
>> 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
>>
>> 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
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> new file mode 100644
>> index 0000000..393b256
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> @@ -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 {
>
> The _tz suffix suggests this is a thermal zone, which isn't the case.
> Call it 'pm7325_temp_alarm' or similar.
>
Okay.
>> + 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 {
>
> 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 {
>
Okay, will change it.
>
>> + 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.
>> + 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.
>
Yeah, this is not required here will remove it.
>> + 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";
>> + };
>
> 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.
>
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.
>> + };
>> + };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> new file mode 100644
>> index 0000000..dffa79d
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
>> @@ -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 ?
>
Ok.
>> + 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.
>> + 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>;
>> + };
>> + };
>> +};
>> +
>> +&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
>
>> +
>> + trips {
>> + pm8350c_trip0: trip0 {
>> + temperature = <95000>;
>> + hysteresis = <0>;
>> + type = "passive";
>> + };
>> +
>> + pm8350c_trip1: trip1 {
>> + temperature = <115000>;
>> + hysteresis = <0>;
>> + type = "critical";
>> + };
>> +
>> + pm8350c_trip2: trip2 {
>> + temperature = <145000>;
>> + hysteresis = <0>;
>> + type = "critical";
>> + };
>
> same as for pm7325
>
>> + };
>> + };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
>> b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
>> new file mode 100644
>> index 0000000..9749484
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
>> @@ -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 {
>> + 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.
>> + 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.
>> + 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>;
>> + };
>> + };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/pmr735a.dtsi
>> b/arch/arm64/boot/dts/qcom/pmr735a.dtsi
>> new file mode 100644
>> index 0000000..e1d2356
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/pmr735a.dtsi
>> @@ -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 {
>> + 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.
>> + 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;
>> + 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
>
>> + polling-delay-passive = <100>;
>> + polling-delay = <0>;
>> + thermal-governor = "step_wise";
>> + thermal-sensors = <&pmr735a_tz>;
>
> same as for pm7325
>
>> +
>> + trips {
>> + pmr735a_trip0: trip0 {
>> + temperature = <95000>;
>> + hysteresis = <0>;
>> + type = "passive";
>> + };
>> +
>> + pmr735a_trip1: trip1 {
>> + temperature = <115000>;
>> + hysteresis = <0>;
>> + type = "critical";
>> + };
>> +
>> + pmr735a_trip2: trip2 {
>> + temperature = <145000>;
>> + hysteresis = <0>;
>> + type = "critical";
>> + };
>
> same as for pm7325
>
>> + };
>> + };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 8af6d77..25402d4 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -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"
Thanks,
Satya Priya
On 2021-03-13 02:17, Matthias Kaehlcke wrote:
> On Thu, Mar 11, 2021 at 04:10:29PM +0530, satya priya wrote:
>> 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
>>
>> 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.
Okay, will split them.
Hi Satya,
On Mon, Mar 22, 2021 at 06:50:47PM +0530, [email protected] wrote:
> Hi Matthias,
>
> On 2021-03-13 02:10, Matthias Kaehlcke wrote:
> > Hi Satya,
> >
> > On Thu, Mar 11, 2021 at 04:10:29PM +0530, satya priya wrote:
> > > 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
> > >
> > > 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
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > > b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > > new file mode 100644
> > > index 0000000..393b256
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > > @@ -0,0 +1,60 @@
> >
> > ...
> >
> > > + 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.
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,
On 2021-03-22 23:04, Matthias Kaehlcke wrote:
> Hi Satya,
>
> On Mon, Mar 22, 2021 at 06:50:47PM +0530, [email protected] wrote:
>> Hi Matthias,
>>
>> On 2021-03-13 02:10, Matthias Kaehlcke wrote:
>> > Hi Satya,
>> >
>> > On Thu, Mar 11, 2021 at 04:10:29PM +0530, satya priya wrote:
>> > > 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
>> > >
>> > > 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
>> > >
>> > > diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> > > b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> > > new file mode 100644
>> > > index 0000000..393b256
>> > > --- /dev/null
>> > > +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> > > @@ -0,0 +1,60 @@
>> >
>> > ...
>> >
>> > > + 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.
>
> 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
On Thu, Mar 25, 2021 at 10:50:57AM +0530, [email protected] wrote:
> Hi Matthias,
>
> On 2021-03-22 23:04, Matthias Kaehlcke wrote:
> > Hi Satya,
> >
> > On Mon, Mar 22, 2021 at 06:50:47PM +0530, [email protected] wrote:
> > > Hi Matthias,
> > >
> > > On 2021-03-13 02:10, Matthias Kaehlcke wrote:
> > > > Hi Satya,
> > > >
> > > > On Thu, Mar 11, 2021 at 04:10:29PM +0530, satya priya wrote:
> > > > > 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
> > > > >
> > > > > 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
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > > > > b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > > > > new file mode 100644
> > > > > index 0000000..393b256
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > > > > @@ -0,0 +1,60 @@
> > > >
> > > > ...
> > > >
> > > > > + 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.
> >
> > 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 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.