Subject: [PATCH 0/2] arm64: dts: qcom: sc8280xp: Make pmic thermal-zones report temperature

The "pm8280-[12]-thermal" thermal zones always report 37C, fix this.

Signed-off-by: Bjorn Andersson <[email protected]>
---
Bjorn Andersson (2):
arm64: dts: qcom: sc8280xp-crd: Add PMIC die-temp vadc channels
arm64: dts: qcom: sc8280xp-pmics: Define adc for temp-alarms

.../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 37 +-------------------
arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 39 +++++++++++++++++++++-
2 files changed, 39 insertions(+), 37 deletions(-)
---
base-commit: 445a555e0623387fa9b94e68e61681717e70200a
change-id: 20240210-sc8280xp-pmic-thermal-fb56e166507e

Best regards,
--
Bjorn Andersson <[email protected]>



Subject: [PATCH 2/2] arm64: dts: qcom: sc8280xp-pmics: Define adc for temp-alarms

From: Bjorn Andersson <[email protected]>

sc8280xp-pmics define the two thermal zones "pm8280-1-thermal" and
"pm8280-2-thermal", but the related temp-alarm instances are not tied to
any adc channels, and as such continuously report the bogus temperature
of 37C.

After previously defining these adc channels across all boards using
sc8280xp-pmics.dtsi, we can now add these references.

This does however mean that we have a non-disabled node referencing
default-disabled nodes, requiring each board to enable the pmk8280_vadc.
Avoid this by marking pmk8280_vadc okay.

Signed-off-by: Bjorn Andersson <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 4 ----
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 2 --
arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 5 ++++-
3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index 22f10813aa3c..ffc4406422ae 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -604,10 +604,6 @@ rtc_offset: rtc-offset@bc {
};
};

-&pmk8280_vadc {
- status = "okay";
-};
-
&qup0 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index ce2564a6c26e..ad42966c94e5 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -851,8 +851,6 @@ rtc_offset: rtc-offset@bc {
};

&pmk8280_vadc {
- status = "okay";
-
channel@144 {
reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
qcom,hw-settle-time = <200>;
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index 58fea376f0e2..945de77911de 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -87,7 +87,6 @@ pmk8280_vadc: adc@3100 {
#address-cells = <1>;
#size-cells = <0>;
#io-channel-cells = <1>;
- status = "disabled";

channel@3 {
reg = <PMK8350_ADC7_DIE_TEMP>;
@@ -160,6 +159,8 @@ pm8280_1_temp_alarm: temp-alarm@a00 {
compatible = "qcom,spmi-temp-alarm";
reg = <0xa00>;
interrupts-extended = <&spmi_bus 0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
+ io-channels = <&pmk8280_vadc PM8350_ADC7_DIE_TEMP(1)>;
+ io-channel-names = "thermal";
#thermal-sensor-cells = <0>;
};

@@ -212,6 +213,8 @@ pm8280_2_temp_alarm: temp-alarm@a00 {
compatible = "qcom,spmi-temp-alarm";
reg = <0xa00>;
interrupts-extended = <&spmi_bus 0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
+ io-channels = <&pmk8280_vadc PM8350_ADC7_DIE_TEMP(3)>;
+ io-channel-names = "thermal";
#thermal-sensor-cells = <0>;
};


--
2.43.0


Subject: [PATCH 1/2] arm64: dts: qcom: sc8280xp-crd: Add PMIC die-temp vadc channels

From: Bjorn Andersson <[email protected]>

The die-temp vadc channels are not defined for the CRD, but describing
them directly would directly duplicate the definition from the Lenovo
Thinkpad X13s DeviceTree.

The sc8280xp-pmics file describes the common configuration of PMK8280,
two PMC8280, PMC8280C, and PMR735a. As such, even though these vadc
channels makes references across PMICs, it's suitable to define them in
the shared file.

Do this, and enable the pmk8280 vadc for the CRD.

Signed-off-by: Bjorn Andersson <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 4 +++
.../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 35 +---------------------
arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 34 +++++++++++++++++++++
3 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index ffc4406422ae..22f10813aa3c 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -604,6 +604,10 @@ rtc_offset: rtc-offset@bc {
};
};

+&pmk8280_vadc {
+ status = "okay";
+};
+
&qup0 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index def3976bd5bb..ce2564a6c26e 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -6,10 +6,8 @@

/dts-v1/;

-#include <dt-bindings/gpio/gpio.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/gpio/gpio.h>
#include <dt-bindings/input/gpio-keys.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/leds/common.h>
@@ -855,25 +853,6 @@ rtc_offset: rtc-offset@bc {
&pmk8280_vadc {
status = "okay";

- channel@3 {
- reg = <PMK8350_ADC7_DIE_TEMP>;
- qcom,pre-scaling = <1 1>;
- label = "pmk8350_die_temp";
- };
-
- channel@44 {
- reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
- qcom,hw-settle-time = <200>;
- qcom,ratiometric;
- label = "pmk8350_xo_therm";
- };
-
- channel@103 {
- reg = <PM8350_ADC7_DIE_TEMP(1)>;
- qcom,pre-scaling = <1 1>;
- label = "pmc8280_1_die_temp";
- };
-
channel@144 {
reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
qcom,hw-settle-time = <200>;
@@ -902,12 +881,6 @@ channel@147 {
label = "sys_therm4";
};

- channel@303 {
- reg = <PM8350_ADC7_DIE_TEMP(3)>;
- qcom,pre-scaling = <1 1>;
- label = "pmc8280_2_die_temp";
- };
-
channel@344 {
reg = <PM8350_ADC7_AMUX_THM1_100K_PU(3)>;
qcom,hw-settle-time = <200>;
@@ -935,12 +908,6 @@ channel@347 {
qcom,ratiometric;
label = "sys_therm8";
};
-
- channel@403 {
- reg = <PMR735A_ADC7_DIE_TEMP>;
- qcom,pre-scaling = <1 1>;
- label = "pmr735a_die_temp";
- };
};

&qup0 {
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index 80ee12ded4f4..58fea376f0e2 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -3,6 +3,9 @@
* Copyright (c) 2022, Linaro Limited
*/

+#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/input/input.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/spmi/spmi.h>
@@ -85,6 +88,37 @@ pmk8280_vadc: adc@3100 {
#size-cells = <0>;
#io-channel-cells = <1>;
status = "disabled";
+
+ channel@3 {
+ reg = <PMK8350_ADC7_DIE_TEMP>;
+ qcom,pre-scaling = <1 1>;
+ label = "pmk8350_die_temp";
+ };
+
+ channel@44 {
+ reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
+ qcom,hw-settle-time = <200>;
+ qcom,ratiometric;
+ label = "pmk8350_xo_therm";
+ };
+
+ channel@103 {
+ reg = <PM8350_ADC7_DIE_TEMP(1)>;
+ qcom,pre-scaling = <1 1>;
+ label = "pmc8280_1_die_temp";
+ };
+
+ channel@303 {
+ reg = <PM8350_ADC7_DIE_TEMP(3)>;
+ qcom,pre-scaling = <1 1>;
+ label = "pmc8280_2_die_temp";
+ };
+
+ channel@403 {
+ reg = <PMR735A_ADC7_DIE_TEMP>;
+ qcom,pre-scaling = <1 1>;
+ label = "pmr735a_die_temp";
+ };
};

pmk8280_adc_tm: adc-tm@3400 {

--
2.43.0


2024-02-12 11:07:59

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sc8280xp-crd: Add PMIC die-temp vadc channels

On 11.02.2024 05:41, Bjorn Andersson via B4 Relay wrote:
> From: Bjorn Andersson <[email protected]>
>
> The die-temp vadc channels are not defined for the CRD, but describing
> them directly would directly duplicate the definition from the Lenovo
> Thinkpad X13s DeviceTree.
>
> The sc8280xp-pmics file describes the common configuration of PMK8280,
> two PMC8280, PMC8280C, and PMR735a. As such, even though these vadc
> channels makes references across PMICs, it's suitable to define them in
> the shared file.

For that reason, you can also keep the VADC enabled by default since
it's a constant element across designs utilizing this set of PMICs

Reviewed-by: Konrad Dybcio <[email protected]>

Konrad

2024-02-12 11:08:57

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sc8280xp-pmics: Define adc for temp-alarms

On 11.02.2024 05:42, Bjorn Andersson via B4 Relay wrote:
> From: Bjorn Andersson <[email protected]>
>
> sc8280xp-pmics define the two thermal zones "pm8280-1-thermal" and
> "pm8280-2-thermal", but the related temp-alarm instances are not tied to
> any adc channels, and as such continuously report the bogus temperature
> of 37C.
>
> After previously defining these adc channels across all boards using
> sc8280xp-pmics.dtsi, we can now add these references.
>
> This does however mean that we have a non-disabled node referencing
> default-disabled nodes, requiring each board to enable the pmk8280_vadc.
> Avoid this by marking pmk8280_vadc okay.

Thanks for addressing my P1 review comments in a way that indicates time
travel has been achieved ;)

Reviewed-by: Konrad Dybcio <[email protected]>

Konrad

2024-02-14 18:03:06

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 0/2] arm64: dts: qcom: sc8280xp: Make pmic thermal-zones report temperature


On Sat, 10 Feb 2024 20:41:58 -0800, Bjorn Andersson wrote:
> The "pm8280-[12]-thermal" thermal zones always report 37C, fix this.
>
>

Applied, thanks!

[1/2] arm64: dts: qcom: sc8280xp-crd: Add PMIC die-temp vadc channels
commit: 6cca740cf345e24fa6bdfeae9236bfb4c6f90c3b
[2/2] arm64: dts: qcom: sc8280xp-pmics: Define adc for temp-alarms
commit: a36a566b5365ab96ed8cfc195478be77811c8708

Best regards,
--
Bjorn Andersson <[email protected]>