Linux Power Management development
 help / color / mirror / Atom feed
* [PATCH v11 0/2] Add support for IPQ5018 tsens
@ 2025-06-11  5:12 George Moussalem via B4 Relay
  2025-06-11  5:12 ` [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible George Moussalem via B4 Relay
  2025-06-11  5:12 ` [PATCH v11 2/2] arm64: dts: qcom: ipq5018: Add tsens node George Moussalem via B4 Relay
  0 siblings, 2 replies; 8+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-11  5:12 UTC (permalink / raw)
  To: Amit Kucheria, Thara Gopinath, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel,
	George Moussalem, Sricharan Ramabadhran, Dmitry Baryshkov

IPQ5018 has tsens V1.0 IP with 5 sensors, of which 4 are in use,
and 1 interrupt. There is no RPM present in the soc to do tsens early
enable. Adding support for the same here.

Last patch series sent by Qualcomm dates back to Sep 22, 2023.
Since I'm working on OpenWrt support for IPQ5018 based boards (routers)
and Sricharan Ramabadhran <quic_srichara@quicinc.com> in below email
confirmed this SoC is still active, I'm continuing the efforts to send
patches upstream for Linux kernel support.
https://lore.kernel.org/all/63dc4054-b1e2-4e7a-94e7-643beb26a6f3@quicinc.com/

[v11]
	*) Dropped polling-delay property as its value of 0 is the default value
	*) Removed comments for TM and SROT in tsens node
	*) Replace underscore by hyphen in node name of top-glue-critical trip
	*) Added cooling device using CPU freq scaling making use of the passive
	   trip defined under the CPU trips
	*) Make qcom,ipq5018-tsens a standalone compatible in the bindings as it
	   should not use qcom,tsens-v1 as a fallback. This also fixes the issue
	   reported by Rob's bot

[v10]
	*) Rebased onto updated pull of master to resolve merge conflicts in the
	   DTS patch
	*) Link to v9: https://lore.kernel.org/all/DS7PR19MB88836DC6965515E12D70BB2C9DCC2@DS7PR19MB8883.namprd19.prod.outlook.com/

[v9]
	*) Updated checks in tsens to more strictly evaluate for v2+ upon enabling
	   v2 features as suggsted by Dmitry.
	*) Split patch 3 into two, one to update conditional statements as
	   mentioned above and the other to implement tsens IP v1 without RPM.
	*) Added back Dmitry's RB tag on patch 6 which wasn't carried over
	   from v7 to v8
	*) Link to v8: https://lore.kernel.org/all/DS7PR19MB88833F7A9C8F4FC484977BA69DCD2@DS7PR19MB8883.namprd19.prod.outlook.com/

[v8]
	*) Tsens V1 uses v1 interrupts and watchdog is not present (only on v2.3+).
	   As such, replaced VER_1_X with VER_1_X_NO_RPM in conditons to ensure
	   v1 interrupts are set and watchdog isn't enabled.
	*) Tested on Linksys MX2000 and SPNMX56
	*) Link to v7: https://lore.kernel.org/all/DS7PR19MB88831624F11516945C63400F9DC22@DS7PR19MB8883.namprd19.prod.outlook.com/

[v7]
	*) Updated cover letter
	*) Replaced patch 3 with a new one to add support for tsens v1.0 with
	   no RPM and removed Dmitry's 'Reviewed-by tag
	*) Refactored patch 4 and split support for IPQ5018 from support for
	   tsens v1.0 without RPM. As such, also removed Dmitry's RB tag.
	*) Depends on patch 1 and 2 from patch series to add support for
	   IQP5332 and IPQ5424 applied on Feb 11 2025:
	   https://patchwork.kernel.org/project/linux-arm-msm/cover/20250210120436.821684-1-quic_mmanikan@quicinc.com/
	*) Link to v6: https://lore.kernel.org/all/DS7PR19MB88838833C0A3BFC3C7FC481F9DC02@DS7PR19MB8883.namprd19.prod.outlook.com/

[v6]
	*) Include (this) cover letter
	*) Picked up Dmitry's Reviewed-by tag on patch 5
	*) Link to v5: https://lore.kernel.org/all/DS7PR19MB88832FDED68D3EBB0EE7E99F9DC72@DS7PR19MB8883.namprd19.prod.outlook.com/

[v5]
	*) Adjusted commit messages to indicate IPQ5018 has 5 sensors of
	   which 4 are described and in use as per downstream driver and dts.
	*) Padded addresses of tsens and qfprom nodes with leading zeros.
	*) Link to v4: https://lore.kernel.org/all/DS7PR19MB8883BE38C2B500D03213747A9DC72@DS7PR19MB8883.namprd19.prod.outlook.com/

[v4]
	*) Documented ipq5018 in qcom,qfprom bindings
	*) Constrained ipq5018-tsens to one interrupt with description
	*) Added Rob's Acked-by tag
	*) Added Dmitry's Reviewed-by tag
	*) Fixed modpost warning: added __init to init_common
	*) Sorted tsens nodes by address
	*) Sorted thermal-zones nodes by name
	*) Link to v3: https://lore.kernel.org/all/20230922115116.2748804-1-srichara@win-platform-upstream01.qualcomm.com/

[v3]
	*) Added the tsens-ipq5018 as  new binding without rpm
	*) Added Dmitry's Reviewed tag
	*) Fixed Dmitry's comments for error checks in init_ipq5018
	*) Ordered the qfprom device node properties
	*) Link to v2: https://lore.kernel.org/all/20230915121504.806672-1-quic_srichara@quicinc.com/

[v2]
	*) Sorted the compatible and removed example
	*) Fixed the name for new tsens_feature
	*) Used tsend_calibrate_common instead of legacy
	   and addressed comments from Dmitry.
	*) Squashed patch 3 & 4
	*) Fixed node names, order and added qfprom cells
            for points seprately
	*) Squashed patch 6 & 7
	*) Link to v1: https://lore.kernel.org/all/1693250307-8910-1-git-send-email-quic_srichara@quicinc.com/

George Moussalem (2):
  thermal: qcom: tsens: update conditions to strictly evaluate for IP
    v2+
  thermal: qcom: tsens: add support for tsens v1 without RPM

Sricharan Ramabadhran (4):
  dt-bindings: nvmem: Add compatible for IPQ5018
  dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible
  thermal: qcom: tsens: Add support for IPQ5018 tsens
  arm64: dts: qcom: ipq5018: Add tsens node

 .../bindings/nvmem/qcom,qfprom.yaml           |   1 +
 .../bindings/thermal/qcom-tsens.yaml          |   2 +
 arch/arm64/boot/dts/qcom/ipq5018.dtsi         | 169 ++++++++++++++++++
 drivers/thermal/qcom/tsens-v1.c               |  62 +++++++
 drivers/thermal/qcom/tsens.c                  |  27 ++-
 drivers/thermal/qcom/tsens.h                  |   4 +
 6 files changed, 256 insertions(+), 9 deletions(-)

--
2.48.1

---
---
George Moussalem (1):
      dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible

Sricharan Ramabadhran (1):
      arm64: dts: qcom: ipq5018: Add tsens node

 .../devicetree/bindings/thermal/qcom-tsens.yaml    |   5 +-
 arch/arm64/boot/dts/qcom/ipq5018.dtsi              | 182 +++++++++++++++++++++
 2 files changed, 186 insertions(+), 1 deletion(-)
---
base-commit: afc582fb6563b8eb5cd73f9eca52e55da827567f
change-id: 20250404-ipq5018-tsens-64bf95fd3317

Best regards,
-- 
George Moussalem <george.moussalem@outlook.com>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible
  2025-06-11  5:12 [PATCH v11 0/2] Add support for IPQ5018 tsens George Moussalem via B4 Relay
@ 2025-06-11  5:12 ` George Moussalem via B4 Relay
  2025-06-11  6:51   ` Krzysztof Kozlowski
  2025-06-11  5:12 ` [PATCH v11 2/2] arm64: dts: qcom: ipq5018: Add tsens node George Moussalem via B4 Relay
  1 sibling, 1 reply; 8+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-11  5:12 UTC (permalink / raw)
  To: Amit Kucheria, Thara Gopinath, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel,
	George Moussalem

From: George Moussalem <george.moussalem@outlook.com>

IPQ5018 tsens should not use qcom,tsens-v1 as fallback since it has no RPM
and, as such, deviates from the standard v1 init routine in the driver.
So let's make qcom,ipq5018-tsens a standalone compatible in the bindings.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -36,10 +36,13 @@ properties:
               - qcom,msm8974-tsens
           - const: qcom,tsens-v0_1
 
+      - description: v1 of TSENS
+        enum:
+          - qcom,ipq5018-tsens
+
       - description: v1 of TSENS
         items:
           - enum:
-              - qcom,ipq5018-tsens
               - qcom,msm8937-tsens
               - qcom,msm8956-tsens
               - qcom,msm8976-tsens

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v11 2/2] arm64: dts: qcom: ipq5018: Add tsens node
  2025-06-11  5:12 [PATCH v11 0/2] Add support for IPQ5018 tsens George Moussalem via B4 Relay
  2025-06-11  5:12 ` [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible George Moussalem via B4 Relay
@ 2025-06-11  5:12 ` George Moussalem via B4 Relay
  1 sibling, 0 replies; 8+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-11  5:12 UTC (permalink / raw)
  To: Amit Kucheria, Thara Gopinath, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel,
	Sricharan Ramabadhran, George Moussalem, Dmitry Baryshkov

From: Sricharan Ramabadhran <quic_srichara@quicinc.com>

IPQ5018 has tsens V1.0 IP with 5 sensors, though 4 are in use.
There is no RPM, so tsens has to be manually enabled. Adding the tsens
and nvmem nodes and adding 4 thermal sensors (zones). The critical trip
temperature is set to 120'C with an action to reboot.

In addition, adding a cooling device to the CPU thermal zone which uses
CPU frequency scaling.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 arch/arm64/boot/dts/qcom/ipq5018.dtsi | 182 ++++++++++++++++++++++++++++++++++
 1 file changed, 182 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 130360014c5e14c778e348d37e601f60325b0b14..defeb697c8d89686e3aaf2e6f7b6cb7493219336 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/qcom,gcc-ipq5018.h>
 #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	interrupt-parent = <&intc>;
@@ -39,6 +40,7 @@ cpu0: cpu@0 {
 			next-level-cache = <&l2_0>;
 			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
 			operating-points-v2 = <&cpu_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu1: cpu@1 {
@@ -49,6 +51,7 @@ cpu1: cpu@1 {
 			next-level-cache = <&l2_0>;
 			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
 			operating-points-v2 = <&cpu_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		l2_0: l2-cache {
@@ -182,6 +185,117 @@ pcie0_phy: phy@86000 {
 			status = "disabled";
 		};
 
+		qfprom: qfprom@a0000 {
+			compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";
+			reg = <0x000a0000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			tsens_mode: mode@249 {
+				reg = <0x249 0x1>;
+				bits = <0 3>;
+			};
+
+			tsens_base1: base1@249 {
+				reg = <0x249 0x2>;
+				bits = <3 8>;
+			};
+
+			tsens_base2: base2@24a {
+				reg = <0x24a 0x2>;
+				bits = <3 8>;
+			};
+
+			tsens_s0_p1: s0-p1@24b {
+				reg = <0x24b 0x2>;
+				bits = <2 6>;
+			};
+
+			tsens_s0_p2: s0-p2@24c {
+				reg = <0x24c 0x1>;
+				bits = <1 6>;
+			};
+
+			tsens_s1_p1: s1-p1@24c {
+				reg = <0x24c 0x2>;
+				bits = <7 6>;
+			};
+
+			tsens_s1_p2: s1-p2@24d {
+				reg = <0x24d 0x2>;
+				bits = <5 6>;
+			};
+
+			tsens_s2_p1: s2-p1@24e {
+				reg = <0x24e 0x2>;
+				bits = <3 6>;
+			};
+
+			tsens_s2_p2: s2-p2@24f {
+				reg = <0x24f 0x1>;
+				bits = <1 6>;
+			};
+
+			tsens_s3_p1: s3-p1@24f {
+				reg = <0x24f 0x2>;
+				bits = <7 6>;
+			};
+
+			tsens_s3_p2: s3-p2@250 {
+				reg = <0x250 0x2>;
+				bits = <5 6>;
+			};
+
+			tsens_s4_p1: s4-p1@251 {
+				reg = <0x251 0x2>;
+				bits = <3 6>;
+			};
+
+			tsens_s4_p2: s4-p2@254 {
+				reg = <0x254 0x1>;
+				bits = <0 6>;
+			};
+		};
+
+		tsens: thermal-sensor@4a9000 {
+			compatible = "qcom,ipq5018-tsens";
+			reg = <0x004a9000 0x1000>,
+			      <0x004a8000 0x1000>;
+
+			nvmem-cells = <&tsens_mode>,
+				      <&tsens_base1>,
+				      <&tsens_base2>,
+				      <&tsens_s0_p1>,
+				      <&tsens_s0_p2>,
+				      <&tsens_s1_p1>,
+				      <&tsens_s1_p2>,
+				      <&tsens_s2_p1>,
+				      <&tsens_s2_p2>,
+				      <&tsens_s3_p1>,
+				      <&tsens_s3_p2>,
+				      <&tsens_s4_p1>,
+				      <&tsens_s4_p2>;
+
+			nvmem-cell-names = "mode",
+					   "base1",
+					   "base2",
+					   "s0_p1",
+					   "s0_p2",
+					   "s1_p1",
+					   "s1_p2",
+					   "s2_p1",
+					   "s2_p2",
+					   "s3_p1",
+					   "s3_p2",
+					   "s4_p1",
+					   "s4_p2";
+
+			interrupts = <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "uplow";
+			#qcom,sensors = <5>;
+			#thermal-sensor-cells = <1>;
+		};
+
 		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq5018-tlmm";
 			reg = <0x01000000 0x300000>;
@@ -631,6 +745,74 @@ pcie@0 {
 		};
 	};
 
+	thermal-zones {
+		cpu-thermal {
+			polling-delay-passive = <0>;
+			thermal-sensors = <&tsens 2>;
+
+			trips {
+				cpu-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+
+				cpu_alert: cpu-passive {
+					temperature = <100000>;
+					hysteresis = <2>;
+					type = "passive";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert>;
+					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+
+		gephy-thermal {
+			polling-delay-passive = <0>;
+			thermal-sensors = <&tsens 4>;
+
+			trips {
+				gephy-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+			};
+		};
+
+		top-glue-thermal {
+			polling-delay-passive = <0>;
+			thermal-sensors = <&tsens 3>;
+
+			trips {
+				top-glue-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+			};
+		};
+
+		ubi32-thermal {
+			polling-delay-passive = <0>;
+			thermal-sensors = <&tsens 1>;
+
+			trips {
+				ubi32-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible
  2025-06-11  5:12 ` [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible George Moussalem via B4 Relay
@ 2025-06-11  6:51   ` Krzysztof Kozlowski
  2025-06-11  7:06     ` George Moussalem
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-11  6:51 UTC (permalink / raw)
  To: george.moussalem, Amit Kucheria, Thara Gopinath,
	Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel

On 11/06/2025 07:12, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> IPQ5018 tsens should not use qcom,tsens-v1 as fallback since it has no RPM
> and, as such, deviates from the standard v1 init routine in the driver.
> So let's make qcom,ipq5018-tsens a standalone compatible in the bindings.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
>  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 5 ++++-

You just added it recently with the fallback (in v9 of this patchset)
and now remove it?

And what does it mean it has no RPM? How does it affect the driver? Does
fallback work or not?


>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -36,10 +36,13 @@ properties:
>                - qcom,msm8974-tsens
>            - const: qcom,tsens-v0_1
>  
> +      - description: v1 of TSENS

So that's still v1... I don't understand.

> +        enum:
> +          - qcom,ipq5018-tsens
> +
>        - description: v1 of TSENS
>          items:
>            - enum:
> -              - qcom,ipq5018-tsens
>                - qcom,msm8937-tsens
>                - qcom,msm8956-tsens
>                - qcom,msm8976-tsens
> 


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible
  2025-06-11  6:51   ` Krzysztof Kozlowski
@ 2025-06-11  7:06     ` George Moussalem
  2025-06-11  7:13       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: George Moussalem @ 2025-06-11  7:06 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Amit Kucheria, Thara Gopinath,
	Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel

Hi Krzysztof,

On 6/11/25 10:51, Krzysztof Kozlowski wrote:
> On 11/06/2025 07:12, George Moussalem via B4 Relay wrote:
>> From: George Moussalem <george.moussalem@outlook.com>
>>
>> IPQ5018 tsens should not use qcom,tsens-v1 as fallback since it has no RPM
>> and, as such, deviates from the standard v1 init routine in the driver.
>> So let's make qcom,ipq5018-tsens a standalone compatible in the bindings.
>>
>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>> ---
>>   Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 5 ++++-
> 
> You just added it recently with the fallback (in v9 of this patchset)
> and now remove it?
> 
> And what does it mean it has no RPM? How does it affect the driver? Does
> fallback work or not?

IPQ5018 tsens IP is V1, but since it's got no RPM, it follows a 
different init routine for which VER_1_X_NO_RPM was created just like 
there is one for V2 without RPM, else the driver wouldn't probe. This 
was added as part of:

https://patchwork.kernel.org/project/linux-arm-msm/patch/DS7PR19MB8883C5D7974C7735E23923769DCC2@DS7PR19MB8883.namprd19.prod.outlook.com/

Since its introduction, I missed updating the bindings which caused a 
binding issue (as reported by Rob) on the compatible as it expects the 
qcom,tsens-v1 as a fallback. But we can't use that fallback, so that's 
why it needs to be a standalone compatible.

> 
> 
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>> index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644
>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>> @@ -36,10 +36,13 @@ properties:
>>                 - qcom,msm8974-tsens
>>             - const: qcom,tsens-v0_1
>>   
>> +      - description: v1 of TSENS
> 
> So that's still v1... I don't understand.

As mentioned, the IP is still v1 but with a different init routine in 
the driver for IP v1 without RPM

> 
>> +        enum:
>> +          - qcom,ipq5018-tsens
>> +
>>         - description: v1 of TSENS
>>           items:
>>             - enum:
>> -              - qcom,ipq5018-tsens
>>                 - qcom,msm8937-tsens
>>                 - qcom,msm8956-tsens
>>                 - qcom,msm8976-tsens
>>
> 
> 
> Best regards,
> Krzysztof

Best regards,
George


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible
  2025-06-11  7:06     ` George Moussalem
@ 2025-06-11  7:13       ` Krzysztof Kozlowski
  2025-06-11  7:23         ` George Moussalem
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-11  7:13 UTC (permalink / raw)
  To: George Moussalem, Amit Kucheria, Thara Gopinath,
	Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel

On 11/06/2025 09:06, George Moussalem wrote:
> Hi Krzysztof,
> 
> On 6/11/25 10:51, Krzysztof Kozlowski wrote:
>> On 11/06/2025 07:12, George Moussalem via B4 Relay wrote:
>>> From: George Moussalem <george.moussalem@outlook.com>
>>>
>>> IPQ5018 tsens should not use qcom,tsens-v1 as fallback since it has no RPM
>>> and, as such, deviates from the standard v1 init routine in the driver.
>>> So let's make qcom,ipq5018-tsens a standalone compatible in the bindings.
>>>
>>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>>> ---
>>>   Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 5 ++++-
>>
>> You just added it recently with the fallback (in v9 of this patchset)
>> and now remove it?
>>
>> And what does it mean it has no RPM? How does it affect the driver? Does
>> fallback work or not?
> 
> IPQ5018 tsens IP is V1, but since it's got no RPM, it follows a 
> different init routine for which VER_1_X_NO_RPM was created just like 
> there is one for V2 without RPM, else the driver wouldn't probe. This 
> was added as part of:
> 
> https://patchwork.kernel.org/project/linux-arm-msm/patch/DS7PR19MB8883C5D7974C7735E23923769DCC2@DS7PR19MB8883.namprd19.prod.outlook.com/
> 
> Since its introduction, I missed updating the bindings which caused a 
> binding issue (as reported by Rob) on the compatible as it expects the 
> qcom,tsens-v1 as a fallback. But we can't use that fallback, so that's 
> why it needs to be a standalone compatible.


So you need Fixes tag and explain what was buggy in previous patch.

> 
>>
>>
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644
>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> @@ -36,10 +36,13 @@ properties:
>>>                 - qcom,msm8974-tsens
>>>             - const: qcom,tsens-v0_1
>>>   
>>> +      - description: v1 of TSENS
>>
>> So that's still v1... I don't understand.
> 
> As mentioned, the IP is still v1 but with a different init routine in 
> the driver for IP v1 without RPM

OK, just merge it into first enum and drop the description there.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible
  2025-06-11  7:13       ` Krzysztof Kozlowski
@ 2025-06-11  7:23         ` George Moussalem
  2025-06-11  7:30           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: George Moussalem @ 2025-06-11  7:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Amit Kucheria, Thara Gopinath,
	Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel



On 6/11/25 11:13, Krzysztof Kozlowski wrote:
> On 11/06/2025 09:06, George Moussalem wrote:
>> Hi Krzysztof,
>>
>> On 6/11/25 10:51, Krzysztof Kozlowski wrote:
>>> On 11/06/2025 07:12, George Moussalem via B4 Relay wrote:
>>>> From: George Moussalem <george.moussalem@outlook.com>
>>>>
>>>> IPQ5018 tsens should not use qcom,tsens-v1 as fallback since it has no RPM
>>>> and, as such, deviates from the standard v1 init routine in the driver.
>>>> So let's make qcom,ipq5018-tsens a standalone compatible in the bindings.
>>>>
>>>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 5 ++++-
>>>
>>> You just added it recently with the fallback (in v9 of this patchset)
>>> and now remove it?
>>>
>>> And what does it mean it has no RPM? How does it affect the driver? Does
>>> fallback work or not?
>>
>> IPQ5018 tsens IP is V1, but since it's got no RPM, it follows a
>> different init routine for which VER_1_X_NO_RPM was created just like
>> there is one for V2 without RPM, else the driver wouldn't probe. This
>> was added as part of:
>>
>> https://patchwork.kernel.org/project/linux-arm-msm/patch/DS7PR19MB8883C5D7974C7735E23923769DCC2@DS7PR19MB8883.namprd19.prod.outlook.com/
>>
>> Since its introduction, I missed updating the bindings which caused a
>> binding issue (as reported by Rob) on the compatible as it expects the
>> qcom,tsens-v1 as a fallback. But we can't use that fallback, so that's
>> why it needs to be a standalone compatible.
> 
> 
> So you need Fixes tag and explain what was buggy in previous patch.

will add, thanks.

> 
>>
>>>
>>>
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>> index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644
>>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>> @@ -36,10 +36,13 @@ properties:
>>>>                  - qcom,msm8974-tsens
>>>>              - const: qcom,tsens-v0_1
>>>>    
>>>> +      - description: v1 of TSENS
>>>
>>> So that's still v1... I don't understand.
>>
>> As mentioned, the IP is still v1 but with a different init routine in
>> the driver for IP v1 without RPM
> 
> OK, just merge it into first enum and drop the description there.

can't merge it into the first enum as that description is invalid for 
this SoC ("description: msm8960 TSENS based").

My proposal would be:

       - description: v1 of TSENS
         oneOf:
           - enum: # for IP V1 without RPM
               - qcom,ipq5018-tsens
           - items:
               - enum:
                   - qcom,msm8937-tsens
                   - qcom,msm8956-tsens
                   - qcom,msm8976-tsens
                   - qcom,qcs404-tsens
               - const: qcom,tsens-v1

thoughts?

> 
> Best regards,
> Krzysztof

Best regards,
George


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible
  2025-06-11  7:23         ` George Moussalem
@ 2025-06-11  7:30           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-11  7:30 UTC (permalink / raw)
  To: George Moussalem, Amit Kucheria, Thara Gopinath,
	Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-pm, devicetree, linux-kernel

On 11/06/2025 09:23, George Moussalem wrote:
>>>>
>>>>
>>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>>> index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644
>>>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>>>> @@ -36,10 +36,13 @@ properties:
>>>>>                  - qcom,msm8974-tsens
>>>>>              - const: qcom,tsens-v0_1
>>>>>    
>>>>> +      - description: v1 of TSENS
>>>>
>>>> So that's still v1... I don't understand.
>>>
>>> As mentioned, the IP is still v1 but with a different init routine in
>>> the driver for IP v1 without RPM
>>
>> OK, just merge it into first enum and drop the description there.
> 
> can't merge it into the first enum as that description is invalid for 
> this SoC ("description: msm8960 TSENS based").

That is why I asked to drop the description there.

> 
> My proposal would be:
> 
>        - description: v1 of TSENS
>          oneOf:
>            - enum: # for IP V1 without RPM
>                - qcom,ipq5018-tsens
>            - items:
>                - enum:
>                    - qcom,msm8937-tsens
>                    - qcom,msm8956-tsens
>                    - qcom,msm8976-tsens
>                    - qcom,qcs404-tsens
>                - const: qcom,tsens-v1

No double nesting. That's already oneOf at the top. Anyway, not
important, so fine if you want to keep it like in this patch, but add
detailed description.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-06-11  7:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11  5:12 [PATCH v11 0/2] Add support for IPQ5018 tsens George Moussalem via B4 Relay
2025-06-11  5:12 ` [PATCH v11 1/2] dt-bindings: thermal: qcom-tsens: make ipq5018 tsens standalone compatible George Moussalem via B4 Relay
2025-06-11  6:51   ` Krzysztof Kozlowski
2025-06-11  7:06     ` George Moussalem
2025-06-11  7:13       ` Krzysztof Kozlowski
2025-06-11  7:23         ` George Moussalem
2025-06-11  7:30           ` Krzysztof Kozlowski
2025-06-11  5:12 ` [PATCH v11 2/2] arm64: dts: qcom: ipq5018: Add tsens node George Moussalem via B4 Relay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox