[PATCH v3 0/4] Enable IPQ9574 TSENS support

[PATCH v3 0/4] Enable IPQ9574 TSENS support

[Varadarajan Narayanan]

This patch set enables tsens in IPQ9574

Depends on
	https://lore.kernel.org/linux-arm-msm/20230406061314.10916-1-quic_devipriy@quicinc.com/
[v3]:
	Fix make DT_CHECKER_FLAGS=-m dt_binding_check and make dtbs_check errors

[v2]:
	Drop the driver change (https://lore.kernel.org/lkml/b45d33d38a334aabbd52c83b0d6028af1f4c74c8.1682682753.git.quic_varada@quicinc.com/)
	since the tsens device is compatible with 8074's tsens
	and use 8074's compatible itself

	Rename clusterX nodes as cpussX

[v1]:
	Fix DT node names

[v0]:
	Initial patch introducing TSENS support

Praveenkumar I (1):
  dt-bindings: thermal: tsens: Add ipq9574 compatible

Varadarajan Narayanan (3):
  dt-bindings: thermal: tsens: Fix "make dtbs_check" error
  arm64: dts: qcom: ipq9574: add tsens node
  arm64: dts: qcom: ipq9574: add thermal zone nodes

 .../devicetree/bindings/thermal/qcom-tsens.yaml    |  15 +-
 arch/arm64/boot/dts/qcom/ipq9574.dtsi              | 217 +++++++++++++++++++++
 2 files changed, 230 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH v3 1/4] dt-bindings: thermal: tsens: Fix "make dtbs_check" error

[Varadarajan Narayanan]

While verifying make dtbs_check for ipq9574, qcm2290-tsens and
sm6375-tsens threw the following errors.
	['qcom,qcm2290-tsens', 'qcom,tsens-v2'] is too long
	...
	['qcom,sm6375-tsens', 'qcom,tsens-v2'] is too long

Fix them by adding their entries.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
[v3]:
	Fix the following 'make dtbs_check' error
		['qcom,qcm2290-tsens', 'qcom,tsens-v2'] is too long
		['qcom,sm6375-tsens', 'qcom,tsens-v2'] is too long
---
 Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index d1ec963..d9aa54c 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -48,6 +48,7 @@ properties:
               - qcom,msm8953-tsens
               - qcom,msm8996-tsens
               - qcom,msm8998-tsens
+              - qcom,qcm2290-tsens
               - qcom,sc7180-tsens
               - qcom,sc7280-tsens
               - qcom,sc8180x-tsens
@@ -56,6 +57,7 @@ properties:
               - qcom,sdm845-tsens
               - qcom,sm6115-tsens
               - qcom,sm6350-tsens
+              - qcom,sm6375-tsens
               - qcom,sm8150-tsens
               - qcom,sm8250-tsens
               - qcom,sm8350-tsens
-- 
2.7.4

[PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Varadarajan Narayanan]

From: Praveenkumar I <quic_ipkumar@quicinc.com>

Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
[v3]:
	Fix dt_binding_check & dtbs_check errors (Used
	Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
	as reference/example)

	Drop 'Acked-by: Rob Herring' as suggested in review

[v2]:
	Thanks to Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
	for the tip to make qcom,ipq8074-tsens as fallback.
---
 Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index d9aa54c..57e3908 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -19,6 +19,11 @@ description: |
 properties:
   compatible:
     oneOf:
+      - const: qcom,tsens-v0_1
+      - const: qcom,tsens-v1
+      - const: qcom,tsens-v2
+      - const: qcom,ipq8074-tsens
+
       - description: msm8960 TSENS based
         items:
           - enum:
@@ -66,8 +71,10 @@ properties:
           - const: qcom,tsens-v2
 
       - description: v2 of TSENS with combined interrupt
-        enum:
-          - qcom,ipq8074-tsens
+        items:
+          - enum:
+              - qcom,ipq9574-tsens
+          - const: qcom,ipq8074-tsens
 
   reg:
     items:
@@ -279,6 +286,7 @@ allOf:
           contains:
             enum:
               - qcom,ipq8074-tsens
+              - qcom,ipq9574-tsens
     then:
       properties:
         interrupts:
@@ -294,6 +302,7 @@ allOf:
           contains:
             enum:
               - qcom,ipq8074-tsens
+              - qcom,ipq9574-tsens
               - qcom,tsens-v0_1
               - qcom,tsens-v1
               - qcom,tsens-v2
-- 
2.7.4

[PATCH v3 3/4] arm64: dts: qcom: ipq9574: add tsens node

[Varadarajan Narayanan]

IPQ9574 has a tsens v2.3.1 peripheral which monitors temperatures
around the various subsystems on the die.

Co-developed-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
[v2]:
	Add "qcom,ipq8074-tsens" as fallback compatible
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 93e3026..85bba6a 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -1056,6 +1056,15 @@
 			status = "disabled";
 		};
 
+		tsens: thermal-sensor@4a9000 {
+			compatible = "qcom,ipq9574-tsens", "qcom,ipq8074-tsens";
+			reg = <0x4a9000 0x1000>, /* TM */
+			      <0x4a8000 0x1000>; /* SROT */
+			interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "combined";
+			#qcom,sensors = <16>;
+			#thermal-sensor-cells = <1>;
+		};
 	};
 
 	timer {
-- 
2.7.4

[PATCH v3 4/4] arm64: dts: qcom: ipq9574: add thermal zone nodes

[Varadarajan Narayanan]

This patch adds thermal zone nodes for the various
sensors present in IPQ9574

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Co-developed-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
[v2]:
	Rename clusterX nodes as cpussX nodes

[v1]:
	Fix node names
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 208 ++++++++++++++++++++++++++++++++++
 1 file changed, 208 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 85bba6a..5a29ee7 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -1067,6 +1067,214 @@
 		};
 	};
 
+	thermal-zones {
+		nss-top-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 3>;
+
+			trips {
+				nss-top-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		ubi-0-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 4>;
+
+			trips {
+				ubi_0-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		ubi-1-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 5>;
+
+			trips {
+				ubi_1-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		ubi-2-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 6>;
+
+			trips {
+				ubi_2-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		ubi-3-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 7>;
+
+			trips {
+				ubi_3-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		cpuss0-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 8>;
+
+			trips {
+				cpu-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		cpuss1-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 9>;
+
+			trips {
+				cpu-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		cpu0-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 10>;
+
+			trips {
+				cpu-critical {
+					temperature = <120000>;
+					hysteresis = <10000>;
+					type = "critical";
+				};
+
+				cpu-passive {
+					temperature = <110000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+			};
+		};
+
+		cpu1-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 11>;
+
+			trips {
+				cpu-critical {
+					temperature = <120000>;
+					hysteresis = <10000>;
+					type = "critical";
+				};
+
+				cpu-passive {
+					temperature = <110000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+			};
+		};
+
+		cpu2-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 12>;
+
+			trips {
+				cpu-critical {
+					temperature = <120000>;
+					hysteresis = <10000>;
+					type = "critical";
+				};
+
+				cpu-passive {
+					temperature = <110000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+			};
+		};
+
+		cpu3-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 13>;
+
+			trips {
+				cpu-critical {
+					temperature = <120000>;
+					hysteresis = <10000>;
+					type = "critical";
+				};
+
+				cpu-passive {
+					temperature = <110000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+			};
+		};
+
+		wcss-phyb-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 14>;
+
+			trips {
+				wcss_phyb-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+
+		top-glue-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 15>;
+
+			trips {
+				top_glue-critical {
+					temperature = <125000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
-- 
2.7.4

Re: [PATCH v3 1/4] dt-bindings: thermal: tsens: Fix "make dtbs_check" error

[Krzysztof Kozlowski]

On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> While verifying make dtbs_check for ipq9574, qcm2290-tsens and
> sm6375-tsens threw the following errors.
> 	['qcom,qcm2290-tsens', 'qcom,tsens-v2'] is too long
> 	...
> 	['qcom,sm6375-tsens', 'qcom,tsens-v2'] is too long

Subject: Fix error can be anything, so this should be specific, e.g.
Document missing QCM2290


https://lore.kernel.org/all/20230314-topic-2290_compats-v1-6-47e26c3c0365@linaro.org/

Best regards,
Krzysztof

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Krzysztof Kozlowski]

On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> From: Praveenkumar I <quic_ipkumar@quicinc.com>
> 
> Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> [v3]:
> 	Fix dt_binding_check & dtbs_check errors (Used
> 	Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> 	as reference/example)
> 
> 	Drop 'Acked-by: Rob Herring' as suggested in review
> 
> [v2]:
> 	Thanks to Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 	for the tip to make qcom,ipq8074-tsens as fallback.
> ---
>  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index d9aa54c..57e3908 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -19,6 +19,11 @@ description: |
>  properties:
>    compatible:
>      oneOf:
> +      - const: qcom,tsens-v0_1
> +      - const: qcom,tsens-v1
> +      - const: qcom,tsens-v2

Nope, these are not correct.

> +      - const: qcom,ipq8074-tsens

Also nope, this is already there.

> +
>        - description: msm8960 TSENS based
>          items:
>            - enum:
> @@ -66,8 +71,10 @@ properties:
>            - const: qcom,tsens-v2
>  
>        - description: v2 of TSENS with combined interrupt
> -        enum:
> -          - qcom,ipq8074-tsens

Why?

> +        items:
> +          - enum:
> +              - qcom,ipq9574-tsens
> +          - const: qcom,ipq8074-tsens
>  
>    reg:
>      items:
> @@ -279,6 +286,7 @@ allOf:
>            contains:
>              enum:
>                - qcom,ipq8074-tsens
> +              - qcom,ipq9574-tsens

Not needed, drop.

>      then:
>        properties:
>          interrupts:
> @@ -294,6 +302,7 @@ allOf:
>            contains:
>              enum:
>                - qcom,ipq8074-tsens
> +              - qcom,ipq9574-tsens

Ditto.


Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: thermal: tsens: Fix "make dtbs_check" error

[Konrad Dybcio]

On 15.05.2023 12:13, Varadarajan Narayanan wrote:
> While verifying make dtbs_check for ipq9574, qcm2290-tsens and
> sm6375-tsens threw the following errors.
> 	['qcom,qcm2290-tsens', 'qcom,tsens-v2'] is too long
> 	...
> 	['qcom,sm6375-tsens', 'qcom,tsens-v2'] is too long
> 
> Fix them by adding their entries.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
https://lore.kernel.org/linux-arm-msm/20230314-topic-2290_compats-v1-6-47e26c3c0365@linaro.org/
https://lore.kernel.org/linux-arm-msm/20230303-topic-sm6375_features0_dts-v2-1-708b8191f7eb@linaro.org/

These never got picked up..

I'll bump them.

Konrad
> [v3]:
> 	Fix the following 'make dtbs_check' error
> 		['qcom,qcm2290-tsens', 'qcom,tsens-v2'] is too long
> 		['qcom,sm6375-tsens', 'qcom,tsens-v2'] is too long
> ---
>  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index d1ec963..d9aa54c 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -48,6 +48,7 @@ properties:
>                - qcom,msm8953-tsens
>                - qcom,msm8996-tsens
>                - qcom,msm8998-tsens
> +              - qcom,qcm2290-tsens
>                - qcom,sc7180-tsens
>                - qcom,sc7280-tsens
>                - qcom,sc8180x-tsens
> @@ -56,6 +57,7 @@ properties:
>                - qcom,sdm845-tsens
>                - qcom,sm6115-tsens
>                - qcom,sm6350-tsens
> +              - qcom,sm6375-tsens
>                - qcom,sm8150-tsens
>                - qcom,sm8250-tsens
>                - qcom,sm8350-tsens

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Varadarajan Narayanan]

On Mon, May 15, 2023 at 06:10:29PM +0200, Krzysztof Kozlowski wrote:
> On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> > From: Praveenkumar I <quic_ipkumar@quicinc.com>
> >
> > Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
> >
> > Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > [v3]:
> > 	Fix dt_binding_check & dtbs_check errors (Used
> > 	Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> > 	as reference/example)
> >
> > 	Drop 'Acked-by: Rob Herring' as suggested in review
> >
> > [v2]:
> > 	Thanks to Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > 	for the tip to make qcom,ipq8074-tsens as fallback.
> > ---
> >  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> > index d9aa54c..57e3908 100644
> > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> > @@ -19,6 +19,11 @@ description: |
> >  properties:
> >    compatible:
> >      oneOf:
> > +      - const: qcom,tsens-v0_1
> > +      - const: qcom,tsens-v1
> > +      - const: qcom,tsens-v2
>
> Nope, these are not correct.
>
> > +      - const: qcom,ipq8074-tsens
>
> Also nope, this is already there.
>
> > +
> >        - description: msm8960 TSENS based
> >          items:
> >            - enum:
> > @@ -66,8 +71,10 @@ properties:
> >            - const: qcom,tsens-v2
> >
> >        - description: v2 of TSENS with combined interrupt
> > -        enum:
> > -          - qcom,ipq8074-tsens
>
> Why?
>
> > +        items:
> > +          - enum:
> > +              - qcom,ipq9574-tsens
> > +          - const: qcom,ipq8074-tsens

Without changing it like this either dtbs_check or
dt_binding_check kept failing.

	- description: v2 of TSENS with combined interrupt
	  enum:
	    - qcom,ipq8074-tsens
	    - qcom,ipq9574-tsens

dtbs_check gave this kind of error
	['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long

After changing it like in https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31

	- description: v2 of TSENS with combined interrupt
	  const: qcom,ipq8074-tsens
	  - enum:
	      - qcom,ipq9574-tsens
	  - const: qcom,ipq8074-tsens

dt_binding_check gives the following error

	Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key

and dtbs_check gives

	./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: [error] syntax error: expected <block end>, but found '-' (syntax)
	  CHKDT   Documentation/devicetree/bindings/processed-schema.json
	./Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
	./Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
	./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
	  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
	/local/mnt/workspace/varada/v3/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml: ignoring, error parsing file

If i change it like below,

	- description: v2 of TSENS with combined interrupt
	  enum:
	    - qcom,ipq9574-tsens
	  - const: qcom,ipq8074-tsens

dt_binding_check and dtbs_check gives same error as above.

Looked around and found Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
which seemed to do something similar to what is wanted in this
case. Hence changed qcom-tsens.yaml similar to the allwinner yaml
file. After which dt_binding_check and dtbs_check passed. Please
let me know if there is a better way to solve this. Will go with
that.

Thanks
Varada

> >
> >    reg:
> >      items:
> > @@ -279,6 +286,7 @@ allOf:
> >            contains:
> >              enum:
> >                - qcom,ipq8074-tsens
> > +              - qcom,ipq9574-tsens
>
> Not needed, drop.
>
> >      then:
> >        properties:
> >          interrupts:
> > @@ -294,6 +302,7 @@ allOf:
> >            contains:
> >              enum:
> >                - qcom,ipq8074-tsens
> > +              - qcom,ipq9574-tsens
>
> Ditto.
>
>
> Best regards,
> Krzysztof
>

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Krzysztof Kozlowski]

On 16/05/2023 14:04, Varadarajan Narayanan wrote:
> On Mon, May 15, 2023 at 06:10:29PM +0200, Krzysztof Kozlowski wrote:
>> On 15/05/2023 12:13, Varadarajan Narayanan wrote:
>>> From: Praveenkumar I <quic_ipkumar@quicinc.com>
>>>
>>> Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
>>>
>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>>> ---
>>> [v3]:
>>> 	Fix dt_binding_check & dtbs_check errors (Used
>>> 	Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
>>> 	as reference/example)
>>>
>>> 	Drop 'Acked-by: Rob Herring' as suggested in review
>>>
>>> [v2]:
>>> 	Thanks to Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> 	for the tip to make qcom,ipq8074-tsens as fallback.
>>> ---
>>>  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> index d9aa54c..57e3908 100644
>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>>> @@ -19,6 +19,11 @@ description: |
>>>  properties:
>>>    compatible:
>>>      oneOf:
>>> +      - const: qcom,tsens-v0_1
>>> +      - const: qcom,tsens-v1
>>> +      - const: qcom,tsens-v2
>>
>> Nope, these are not correct.
>>
>>> +      - const: qcom,ipq8074-tsens
>>
>> Also nope, this is already there.
>>
>>> +
>>>        - description: msm8960 TSENS based
>>>          items:
>>>            - enum:
>>> @@ -66,8 +71,10 @@ properties:
>>>            - const: qcom,tsens-v2
>>>
>>>        - description: v2 of TSENS with combined interrupt
>>> -        enum:
>>> -          - qcom,ipq8074-tsens
>>
>> Why?
>>
>>> +        items:
>>> +          - enum:
>>> +              - qcom,ipq9574-tsens
>>> +          - const: qcom,ipq8074-tsens
> 
> Without changing it like this either dtbs_check or
> dt_binding_check kept failing.
> 
> 	- description: v2 of TSENS with combined interrupt
> 	  enum:
> 	    - qcom,ipq8074-tsens
> 	    - qcom,ipq9574-tsens

But we do not talk about this... Look, I commented out under specific
hunks which are not correct. Not under the hunk which is correct.

> 
> dtbs_check gave this kind of error
> 	['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
> 
> After changing it like in https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
> 
> 	- description: v2 of TSENS with combined interrupt
> 	  const: qcom,ipq8074-tsens
> 	  - enum:
> 	      - qcom,ipq9574-tsens
> 	  - const: qcom,ipq8074-tsens
> 
> dt_binding_check gives the following error
> 
> 	Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key

Because it is not even valid syntax.

> 
> and dtbs_check gives
> 
> 	./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: [error] syntax error: expected <block end>, but found '-' (syntax)
> 	  CHKDT   Documentation/devicetree/bindings/processed-schema.json
> 	./Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> 	./Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> 	./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
> 	  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> 	/local/mnt/workspace/varada/v3/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml: ignoring, error parsing file
> 
> If i change it like below,
> 
> 	- description: v2 of TSENS with combined interrupt
> 	  enum:
> 	    - qcom,ipq9574-tsens
> 	  - const: qcom,ipq8074-tsens
> 
> dt_binding_check and dtbs_check gives same error as above.
> 
> Looked around and found Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> which seemed to do something similar to what is wanted in this
> case. Hence changed qcom-tsens.yaml similar to the allwinner yaml
> file. After which dt_binding_check and dtbs_check passed. Please
> let me know if there is a better way to solve this. Will go with

Changing one valid syntax to another valid syntax is not related to the
patch. If you think such change as reasonable, please split it, but to
me it does not look justified. As for actual change, so adding new
compatible, it's not really related to the others. Why you cannot add
the proper list (so the only valid hunk) and that's it?

Best regards,
Krzysztof

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Varadarajan Narayanan]

On Tue, May 16, 2023 at 03:06:40PM +0200, Krzysztof Kozlowski wrote:
> On 16/05/2023 14:04, Varadarajan Narayanan wrote:
> > On Mon, May 15, 2023 at 06:10:29PM +0200, Krzysztof Kozlowski wrote:
> >> On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> >>> From: Praveenkumar I <quic_ipkumar@quicinc.com>
> >>>
> >>> Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
> >>>
> >>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> >>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> >>> ---
> >>> [v3]:
> >>> 	Fix dt_binding_check & dtbs_check errors (Used
> >>> 	Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> >>> 	as reference/example)
> >>>
> >>> 	Drop 'Acked-by: Rob Herring' as suggested in review
> >>>
> >>> [v2]:
> >>> 	Thanks to Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>> 	for the tip to make qcom,ipq8074-tsens as fallback.
> >>> ---
> >>>  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
> >>>  1 file changed, 11 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> index d9aa54c..57e3908 100644
> >>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> @@ -19,6 +19,11 @@ description: |
> >>>  properties:
> >>>    compatible:
> >>>      oneOf:
> >>> +      - const: qcom,tsens-v0_1
> >>> +      - const: qcom,tsens-v1
> >>> +      - const: qcom,tsens-v2
> >>
> >> Nope, these are not correct.
> >>
> >>> +      - const: qcom,ipq8074-tsens
> >>
> >> Also nope, this is already there.
> >>
> >>> +
> >>>        - description: msm8960 TSENS based
> >>>          items:
> >>>            - enum:
> >>> @@ -66,8 +71,10 @@ properties:
> >>>            - const: qcom,tsens-v2
> >>>
> >>>        - description: v2 of TSENS with combined interrupt
> >>> -        enum:
> >>> -          - qcom,ipq8074-tsens
> >>
> >> Why?
> >>
> >>> +        items:
> >>> +          - enum:
> >>> +              - qcom,ipq9574-tsens
> >>> +          - const: qcom,ipq8074-tsens
> >
> > Without changing it like this either dtbs_check or
> > dt_binding_check kept failing.
> >
> > 	- description: v2 of TSENS with combined interrupt
> > 	  enum:
> > 	    - qcom,ipq8074-tsens
> > 	    - qcom,ipq9574-tsens
>
> But we do not talk about this... Look, I commented out under specific
> hunks which are not correct. Not under the hunk which is correct.
>
> >
> > dtbs_check gave this kind of error
> > 	['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
> >
> > After changing it like in https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
> >
> > 	- description: v2 of TSENS with combined interrupt
> > 	  const: qcom,ipq8074-tsens
> > 	  - enum:
> > 	      - qcom,ipq9574-tsens
> > 	  - const: qcom,ipq8074-tsens
> >
> > dt_binding_check gives the following error
> >
> > 	Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
>
> Because it is not even valid syntax.
>
> >
> > and dtbs_check gives
> >
> > 	./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: [error] syntax error: expected <block end>, but found '-' (syntax)
> > 	  CHKDT   Documentation/devicetree/bindings/processed-schema.json
> > 	./Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> > 	./Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> > 	./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
> > 	  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> > 	/local/mnt/workspace/varada/v3/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml: ignoring, error parsing file
> >
> > If i change it like below,
> >
> > 	- description: v2 of TSENS with combined interrupt
> > 	  enum:
> > 	    - qcom,ipq9574-tsens
> > 	  - const: qcom,ipq8074-tsens
> >
> > dt_binding_check and dtbs_check gives same error as above.
> >
> > Looked around and found Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> > which seemed to do something similar to what is wanted in this
> > case. Hence changed qcom-tsens.yaml similar to the allwinner yaml
> > file. After which dt_binding_check and dtbs_check passed. Please
> > let me know if there is a better way to solve this. Will go with
>
> Changing one valid syntax to another valid syntax is not related to the
> patch. If you think such change as reasonable, please split it, but to
> me it does not look justified. As for actual change, so adding new
> compatible, it's not really related to the others. Why you cannot add
> the proper list (so the only valid hunk) and that's it?

Not sure if I didn't express properly. There are two parts to this patch.

Part-1 is adding the 'const' entries at the beginning i.e.

	+      - const: qcom,tsens-v0_1
	+      - const: qcom,tsens-v1
	+      - const: qcom,tsens-v2
	+      - const: qcom,ipq8074-tsens

Part-2 is changing from one valid syntax to another i.e.

	+        items:
	+          - enum:
	+              - qcom,ipq9574-tsens
	+          - const: qcom,ipq8074-tsens

Without both of the above changes, either or both of dtbs_check
& dt_binding_check fails. So, it is not possible to just add the
"valid hunk" (part-2) alone.

If having both part-1 and part-2 in the same patch is not
acceptable, shall I split them into two patches? Please let me know.

Thanks
Varada

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Krzysztof Kozlowski]

On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> Part-1 is adding the 'const' entries at the beginning i.e.
> 
> 	+      - const: qcom,tsens-v0_1
> 	+      - const: qcom,tsens-v1
> 	+      - const: qcom,tsens-v2
> 	+      - const: qcom,ipq8074-tsens
> 
> Part-2 is changing from one valid syntax to another i.e.
> 
> 	+        items:
> 	+          - enum:
> 	+              - qcom,ipq9574-tsens
> 	+          - const: qcom,ipq8074-tsens
> 
> Without both of the above changes, either or both of dtbs_check
> & dt_binding_check fails. So, it is not possible to just add the
> "valid hunk" (part-2) alone.

Of course it is. All schema files work like that...
> 
> If having both part-1 and part-2 in the same patch is not
> acceptable, shall I split them into two patches? Please let me know.

No, hunk one is not justified.

Best regards,
Krzysztof

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Varadarajan Narayanan]

On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> > Part-1 is adding the 'const' entries at the beginning i.e.
> >
> > 	+      - const: qcom,tsens-v0_1
> > 	+      - const: qcom,tsens-v1
> > 	+      - const: qcom,tsens-v2
> > 	+      - const: qcom,ipq8074-tsens
> >
> > Part-2 is changing from one valid syntax to another i.e.
> >
> > 	+        items:
> > 	+          - enum:
> > 	+              - qcom,ipq9574-tsens
> > 	+          - const: qcom,ipq8074-tsens
> >
> > Without both of the above changes, either or both of dtbs_check
> > & dt_binding_check fails. So, it is not possible to just add the
> > "valid hunk" (part-2) alone.
>
> Of course it is. All schema files work like that...
> >
> > If having both part-1 and part-2 in the same patch is not
> > acceptable, shall I split them into two patches? Please let me know.
>
> No, hunk one is not justified.

For the other compatibles, the enum entries and const/fallback
entries are different. For the 9574 & 8074 case, we want to have
qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
if we don't have the first hunk, dtbs_check fails for 8074
related dtbs

	ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
		['qcom,ipq8074-tsens'] is too short

	ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
		['qcom,ipq8074-tsens'] is too short

	ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
		['qcom,ipq8074-tsens'] is too short

I'm not sure of the correct solution. Having the first hunk
solves the above dtbs_check errors, so went with it. I'm able to
avoid dtbs_check errors with just one entry in the first hunk.

 	+      - const: qcom,ipq8074-tsens

Please let me know if there is a better way to resolve this or we
can have just the 8074 entry in the first hunk.

Thanks
Varada

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Krzysztof Kozlowski]

On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
>>> Part-1 is adding the 'const' entries at the beginning i.e.
>>>
>>> 	+      - const: qcom,tsens-v0_1
>>> 	+      - const: qcom,tsens-v1
>>> 	+      - const: qcom,tsens-v2
>>> 	+      - const: qcom,ipq8074-tsens
>>>
>>> Part-2 is changing from one valid syntax to another i.e.
>>>
>>> 	+        items:
>>> 	+          - enum:
>>> 	+              - qcom,ipq9574-tsens
>>> 	+          - const: qcom,ipq8074-tsens
>>>
>>> Without both of the above changes, either or both of dtbs_check
>>> & dt_binding_check fails. So, it is not possible to just add the
>>> "valid hunk" (part-2) alone.
>>
>> Of course it is. All schema files work like that...
>>>
>>> If having both part-1 and part-2 in the same patch is not
>>> acceptable, shall I split them into two patches? Please let me know.
>>
>> No, hunk one is not justified.
> 
> For the other compatibles, the enum entries and const/fallback
> entries are different. For the 9574 & 8074 case, we want to have
> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
> if we don't have the first hunk, dtbs_check fails for 8074
> related dtbs
> 
> 	ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> 		['qcom,ipq8074-tsens'] is too short

Why? It is already there. Open the file and you will see that this is
already covered.

If you remove it, then yes, you will see errors and the answer is: do
not remove it.

> 
> 	ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> 		['qcom,ipq8074-tsens'] is too short
> 
> 	ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> 		['qcom,ipq8074-tsens'] is too short
> 
> I'm not sure of the correct solution. Having the first hunk
> solves the above dtbs_check errors, so went with it. I'm able to
> avoid dtbs_check errors with just one entry in the first hunk.

You made multiple changes in one patch which is not correct. Your goal
is to add only one change - ipq9574 followed by ipq8074. Add this one.
Don't touch others.

> 
>  	+      - const: qcom,ipq8074-tsens
> 
> Please let me know if there is a better way to resolve this or we
> can have just the 8074 entry in the first hunk.

You only need to add new item on the oneOf list:
 - enum
     - ipq9574
 - const: ipq8074

Best regards,
Krzysztof

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Varadarajan Narayanan]

[-- Attachment #1: Type: text/plain, Size: 3078 bytes --]

On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> > On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> >> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> >>> Part-1 is adding the 'const' entries at the beginning i.e.
> >>>
> >>> 	+      - const: qcom,tsens-v0_1
> >>> 	+      - const: qcom,tsens-v1
> >>> 	+      - const: qcom,tsens-v2
> >>> 	+      - const: qcom,ipq8074-tsens
> >>>
> >>> Part-2 is changing from one valid syntax to another i.e.
> >>>
> >>> 	+        items:
> >>> 	+          - enum:
> >>> 	+              - qcom,ipq9574-tsens
> >>> 	+          - const: qcom,ipq8074-tsens
> >>>
> >>> Without both of the above changes, either or both of dtbs_check
> >>> & dt_binding_check fails. So, it is not possible to just add the
> >>> "valid hunk" (part-2) alone.
> >>
> >> Of course it is. All schema files work like that...
> >>>
> >>> If having both part-1 and part-2 in the same patch is not
> >>> acceptable, shall I split them into two patches? Please let me know.
> >>
> >> No, hunk one is not justified.
> >
> > For the other compatibles, the enum entries and const/fallback
> > entries are different. For the 9574 & 8074 case, we want to have
> > qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
> > if we don't have the first hunk, dtbs_check fails for 8074
> > related dtbs
> >
> > 	ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> > 		['qcom,ipq8074-tsens'] is too short
>
> Why? It is already there. Open the file and you will see that this is
> already covered.

I guess dtbs_check doesn't like the same value being a const and
a oneof entry. Have attached the file, please see if something is
not in order.

> If you remove it, then yes, you will see errors and the answer is: do
> not remove it.

I haven't removed it. For this patch, ipq8074-tsens changed from
being an oneof enum entry to a const entry. Probably, that is why
dtbs_check is giving these errors.

> > 	ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> > 		['qcom,ipq8074-tsens'] is too short
> >
> > 	ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> > 		['qcom,ipq8074-tsens'] is too short
> >
> > I'm not sure of the correct solution. Having the first hunk
> > solves the above dtbs_check errors, so went with it. I'm able to
> > avoid dtbs_check errors with just one entry in the first hunk.
>
> You made multiple changes in one patch which is not correct. Your goal
> is to add only one change - ipq9574 followed by ipq8074. Add this one.
> Don't touch others.

But that breaks dtbs_check.

> >  	+      - const: qcom,ipq8074-tsens
> >
> > Please let me know if there is a better way to resolve this or we
> > can have just the 8074 entry in the first hunk.
>
> You only need to add new item on the oneOf list:
>  - enum
>      - ipq9574
>  - const: ipq8074

The "['qcom,ipq8074-tsens'] is too short" errors were generated
with the above snippet only. Please see the attachment

Thanks
Varada

[-- Attachment #2: qcom-tsens.yaml --]
[-- Type: text/plain, Size: 12313 bytes --]

# SPDX-License-Identifier: (GPL-2.0 OR MIT)
# Copyright 2019 Linaro Ltd.
%YAML 1.2
---
$id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

title: QCOM SoC Temperature Sensor (TSENS)

maintainers:
  - Amit Kucheria <amitk@kernel.org>

description: |
  QCOM SoCs have TSENS IP to allow temperature measurement. There are currently
  three distinct major versions of the IP that is supported by a single driver.
  The IP versions are named v0.1, v1 and v2 in the driver, where v0.1 captures
  everything before v1 when there was no versioning information.

properties:
  compatible:
    oneOf:
      - description: msm8960 TSENS based
        items:
          - enum:
              - qcom,ipq8064-tsens
              - qcom,msm8960-tsens

      - description: v0.1 of TSENS
        items:
          - enum:
              - qcom,mdm9607-tsens
              - qcom,msm8916-tsens
              - qcom,msm8939-tsens
              - qcom,msm8974-tsens
          - const: qcom,tsens-v0_1

      - description: v1 of TSENS
        items:
          - enum:
              - qcom,msm8956-tsens
              - qcom,msm8976-tsens
              - qcom,qcs404-tsens
          - const: qcom,tsens-v1

      - description: v2 of TSENS
        items:
          - enum:
              - qcom,msm8953-tsens
              - qcom,msm8996-tsens
              - qcom,msm8998-tsens
              - qcom,sc7180-tsens
              - qcom,sc7280-tsens
              - qcom,sc8180x-tsens
              - qcom,sc8280xp-tsens
              - qcom,sdm630-tsens
              - qcom,sdm845-tsens
              - qcom,sm6115-tsens
              - qcom,sm6350-tsens
              - qcom,sm8150-tsens
              - qcom,sm8250-tsens
              - qcom,sm8350-tsens
              - qcom,sm8450-tsens
              - qcom,sm8550-tsens
          - const: qcom,tsens-v2

      - description: v2 of TSENS with combined interrupt
        items:
          - enum:
              - qcom,ipq9574-tsens
          - const: qcom,ipq8074-tsens

  reg:
    items:
      - description: TM registers
      - description: SROT registers

  interrupts:
    minItems: 1
    maxItems: 2

  interrupt-names:
    minItems: 1
    maxItems: 2

  nvmem-cells:
    oneOf:
      - minItems: 1
        maxItems: 2
        description:
          Reference to an nvmem node for the calibration data
      - minItems: 5
        maxItems: 35
        description: |
          Reference to nvmem cells for the calibration mode, two calibration
          bases and two cells per each sensor
        # special case for msm8974 / apq8084
      - maxItems: 51
        description: |
          Reference to nvmem cells for the calibration mode, two calibration
          bases and two cells per each sensor, main and backup copies, plus use_backup cell

  nvmem-cell-names:
    oneOf:
      - minItems: 1
        items:
          - const: calib
          - enum:
              - calib_backup
              - calib_sel
      - minItems: 5
        items:
          - const: mode
          - const: base1
          - const: base2
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
          - pattern: '^s[0-9]+_p1$'
          - pattern: '^s[0-9]+_p2$'
        # special case for msm8974 / apq8084
      - items:
          - const: mode
          - const: base1
          - const: base2
          - const: use_backup
          - const: mode_backup
          - const: base1_backup
          - const: base2_backup
          - const: s0_p1
          - const: s0_p2
          - const: s1_p1
          - const: s1_p2
          - const: s2_p1
          - const: s2_p2
          - const: s3_p1
          - const: s3_p2
          - const: s4_p1
          - const: s4_p2
          - const: s5_p1
          - const: s5_p2
          - const: s6_p1
          - const: s6_p2
          - const: s7_p1
          - const: s7_p2
          - const: s8_p1
          - const: s8_p2
          - const: s9_p1
          - const: s9_p2
          - const: s10_p1
          - const: s10_p2
          - const: s0_p1_backup
          - const: s0_p2_backup
          - const: s1_p1_backup
          - const: s1_p2_backup
          - const: s2_p1_backup
          - const: s2_p2_backup
          - const: s3_p1_backup
          - const: s3_p2_backup
          - const: s4_p1_backup
          - const: s4_p2_backup
          - const: s5_p1_backup
          - const: s5_p2_backup
          - const: s6_p1_backup
          - const: s6_p2_backup
          - const: s7_p1_backup
          - const: s7_p2_backup
          - const: s8_p1_backup
          - const: s8_p2_backup
          - const: s9_p1_backup
          - const: s9_p2_backup
          - const: s10_p1_backup
          - const: s10_p2_backup

  "#qcom,sensors":
    description:
      Number of sensors enabled on this platform
    $ref: /schemas/types.yaml#/definitions/uint32
    minimum: 1
    maximum: 16

  "#thermal-sensor-cells":
    const: 1
    description:
      Number of cells required to uniquely identify the thermal sensors. Since
      we have multiple sensors this is set to 1

required:
  - compatible
  - interrupts
  - interrupt-names
  - "#thermal-sensor-cells"
  - "#qcom,sensors"

allOf:
  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,ipq8064-tsens
              - qcom,mdm9607-tsens
              - qcom,msm8916-tsens
              - qcom,msm8960-tsens
              - qcom,msm8974-tsens
              - qcom,msm8976-tsens
              - qcom,qcs404-tsens
              - qcom,tsens-v0_1
              - qcom,tsens-v1
    then:
      properties:
        interrupts:
          items:
            - description: Combined interrupt if upper or lower threshold crossed
        interrupt-names:
          items:
            - const: uplow

  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,msm8953-tsens
              - qcom,msm8996-tsens
              - qcom,msm8998-tsens
              - qcom,sc7180-tsens
              - qcom,sc7280-tsens
              - qcom,sc8180x-tsens
              - qcom,sc8280xp-tsens
              - qcom,sdm630-tsens
              - qcom,sdm845-tsens
              - qcom,sm6350-tsens
              - qcom,sm8150-tsens
              - qcom,sm8250-tsens
              - qcom,sm8350-tsens
              - qcom,sm8450-tsens
              - qcom,tsens-v2
    then:
      properties:
        interrupts:
          items:
            - description: Combined interrupt if upper or lower threshold crossed
            - description: Interrupt if critical threshold crossed
        interrupt-names:
          items:
            - const: uplow
            - const: critical

  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,ipq8074-tsens
    then:
      properties:
        interrupts:
          items:
            - description: Combined interrupt if upper, lower or critical thresholds crossed
        interrupt-names:
          items:
            - const: combined

  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,ipq8074-tsens
              - qcom,tsens-v0_1
              - qcom,tsens-v1
              - qcom,tsens-v2

    then:
      required:
        - reg

additionalProperties: false

examples:
  - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>
    // Example msm9860 based SoC (ipq8064):
    gcc: clock-controller {

           /* ... */

           tsens: thermal-sensor {
                compatible = "qcom,ipq8064-tsens";

                 nvmem-cells = <&tsens_calib>, <&tsens_calib_backup>;
                 nvmem-cell-names = "calib", "calib_backup";
                 interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
                 interrupt-names = "uplow";

                 #qcom,sensors = <11>;
                 #thermal-sensor-cells = <1>;
          };
    };

  - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>
    // Example 1 (new calbiration data: for pre v1 IP):
    thermal-sensor@900000 {
        compatible = "qcom,msm8916-tsens", "qcom,tsens-v0_1";
        reg = <0x4a9000 0x1000>, /* TM */
              <0x4a8000 0x1000>; /* SROT */

        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_s4_p1>, <&tsens_s4_p2>,
                      <&tsens_s5_p1>, <&tsens_s5_p2>;
        nvmem-cell-names = "mode",
                           "base1", "base2",
                           "s0_p1", "s0_p2",
                           "s1_p1", "s1_p2",
                           "s2_p1", "s2_p2",
                           "s4_p1", "s4_p2",
                           "s5_p1", "s5_p2";

        interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
        interrupt-names = "uplow";

        #qcom,sensors = <5>;
        #thermal-sensor-cells = <1>;
    };

  - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>
    // Example 1 (legacy: for pre v1 IP):
    tsens1: thermal-sensor@900000 {
           compatible = "qcom,msm8916-tsens", "qcom,tsens-v0_1";
           reg = <0x4a9000 0x1000>, /* TM */
                 <0x4a8000 0x1000>; /* SROT */

           nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
           nvmem-cell-names = "calib", "calib_sel";

           interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
           interrupt-names = "uplow";

           #qcom,sensors = <5>;
           #thermal-sensor-cells = <1>;
    };

  - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>
    // Example 2 (for any platform containing v1 of the TSENS IP):
    tsens2: thermal-sensor@4a9000 {
          compatible = "qcom,qcs404-tsens", "qcom,tsens-v1";
          reg = <0x004a9000 0x1000>, /* TM */
                <0x004a8000 0x1000>; /* SROT */

          nvmem-cells = <&tsens_caldata>;
          nvmem-cell-names = "calib";

          interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
          interrupt-names = "uplow";

          #qcom,sensors = <10>;
          #thermal-sensor-cells = <1>;
    };

  - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>
    // Example 3 (for any platform containing v2 of the TSENS IP):
    tsens3: thermal-sensor@c263000 {
           compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
           reg = <0xc263000 0x1ff>,
                 <0xc222000 0x1ff>;

           interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
                        <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
           interrupt-names = "uplow", "critical";

           #qcom,sensors = <13>;
           #thermal-sensor-cells = <1>;
    };

  - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>
    // Example 4 (for any IPQ8074 based SoC-s):
    tsens4: thermal-sensor@4a9000 {
           compatible = "qcom,ipq8074-tsens";
           reg = <0x4a9000 0x1000>,
                 <0x4a8000 0x1000>;

           interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
           interrupt-names = "combined";

           #qcom,sensors = <16>;
           #thermal-sensor-cells = <1>;
    };
...

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Krzysztof Kozlowski]

On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
>> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
>>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
>>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
>>>>> Part-1 is adding the 'const' entries at the beginning i.e.
>>>>>
>>>>> 	+      - const: qcom,tsens-v0_1
>>>>> 	+      - const: qcom,tsens-v1
>>>>> 	+      - const: qcom,tsens-v2
>>>>> 	+      - const: qcom,ipq8074-tsens
>>>>>
>>>>> Part-2 is changing from one valid syntax to another i.e.
>>>>>
>>>>> 	+        items:
>>>>> 	+          - enum:
>>>>> 	+              - qcom,ipq9574-tsens
>>>>> 	+          - const: qcom,ipq8074-tsens
>>>>>
>>>>> Without both of the above changes, either or both of dtbs_check
>>>>> & dt_binding_check fails. So, it is not possible to just add the
>>>>> "valid hunk" (part-2) alone.
>>>>
>>>> Of course it is. All schema files work like that...
>>>>>
>>>>> If having both part-1 and part-2 in the same patch is not
>>>>> acceptable, shall I split them into two patches? Please let me know.
>>>>
>>>> No, hunk one is not justified.
>>>
>>> For the other compatibles, the enum entries and const/fallback
>>> entries are different. For the 9574 & 8074 case, we want to have
>>> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
>>> if we don't have the first hunk, dtbs_check fails for 8074
>>> related dtbs
>>>
>>> 	ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>> 		['qcom,ipq8074-tsens'] is too short
>>
>> Why? It is already there. Open the file and you will see that this is
>> already covered.
> 
> I guess dtbs_check doesn't like the same value being a const and
> a oneof entry.

I don't understand.

>  Have attached the file, please see if something is
> not in order.

I don't know what changed there. Please work on patches.

> 
>> If you remove it, then yes, you will see errors and the answer is: do
>> not remove it.
> 
> I haven't removed it. 

You did. Look:

       - description: v2 of TSENS with combined interrupt
-        enum:
-          - qcom,ipq8074-tsens

The first character in the diff (-) means removal.

> For this patch, ipq8074-tsens changed from
> being an oneof enum entry to a const entry. Probably, that is why
> dtbs_check is giving these errors.

You removed the entry which you should not have touched.

> 
>>> 	ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>> 		['qcom,ipq8074-tsens'] is too short
>>>
>>> 	ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>> 		['qcom,ipq8074-tsens'] is too short
>>>
>>> I'm not sure of the correct solution. Having the first hunk
>>> solves the above dtbs_check errors, so went with it. I'm able to
>>> avoid dtbs_check errors with just one entry in the first hunk.
>>
>> You made multiple changes in one patch which is not correct. Your goal
>> is to add only one change - ipq9574 followed by ipq8074. Add this one.
>> Don't touch others.
> 
> But that breaks dtbs_check.

All other cases, hundreds of other binding files, do not have problem.
Only this one "breaks dtbs_check". No, it does not.

Whatever is broken is result of your removal of unrelated pieces.

> 
>>>  	+      - const: qcom,ipq8074-tsens
>>>
>>> Please let me know if there is a better way to resolve this or we
>>> can have just the 8074 entry in the first hunk.
>>
>> You only need to add new item on the oneOf list:
>>  - enum
>>      - ipq9574
>>  - const: ipq8074
> 
> The "['qcom,ipq8074-tsens'] is too short" errors were generated
> with the above snippet only. Please see the attachment

It's not true. The error you see is result because you removed something
you should not. I did not ask you to remove anything. So repeating -
"add new item". Adding is not "removal and adding". Adding is just "adding".

Best regards,
Krzysztof

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Varadarajan Narayanan]

On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
> On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> > On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> >> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> >>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> >>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> >>>>> Part-1 is adding the 'const' entries at the beginning i.e.
> >>>>>
> >>>>> 	+      - const: qcom,tsens-v0_1
> >>>>> 	+      - const: qcom,tsens-v1
> >>>>> 	+      - const: qcom,tsens-v2
> >>>>> 	+      - const: qcom,ipq8074-tsens
> >>>>>
> >>>>> Part-2 is changing from one valid syntax to another i.e.
> >>>>>
> >>>>> 	+        items:
> >>>>> 	+          - enum:
> >>>>> 	+              - qcom,ipq9574-tsens
> >>>>> 	+          - const: qcom,ipq8074-tsens
> >>>>>
> >>>>> Without both of the above changes, either or both of dtbs_check
> >>>>> & dt_binding_check fails. So, it is not possible to just add the
> >>>>> "valid hunk" (part-2) alone.
> >>>>
> >>>> Of course it is. All schema files work like that...
> >>>>>
> >>>>> If having both part-1 and part-2 in the same patch is not
> >>>>> acceptable, shall I split them into two patches? Please let me know.
> >>>>
> >>>> No, hunk one is not justified.
> >>>
> >>> For the other compatibles, the enum entries and const/fallback
> >>> entries are different. For the 9574 & 8074 case, we want to have
> >>> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
> >>> if we don't have the first hunk, dtbs_check fails for 8074
> >>> related dtbs
> >>>
> >>> 	ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> >>> 		['qcom,ipq8074-tsens'] is too short
> >>
> >> Why? It is already there. Open the file and you will see that this is
> >> already covered.
> >
> > I guess dtbs_check doesn't like the same value being a const and
> > a oneof entry.
>
> I don't understand.

      - description: v2 of TSENS with combined interrupt
        items:
          - enum:
              - qcom,ipq9574-tsens	<--- one of the compatible entries
          - const: qcom,ipq8074-tsens	<--- fallback entry

In this patch, we want 8074 to act as a compatible entry for
ipq8074*.dts and fallback entry for ipq9574.dtsi. That is why I
believe we are not able to just add 9574 and get it to pass
dtbs_check and dt_binding_check.

> >  Have attached the file, please see if something is
> > not in order.
>
> I don't know what changed there. Please work on patches.
>
> >
> >> If you remove it, then yes, you will see errors and the answer is: do
> >> not remove it.
> >
> > I haven't removed it.
>
> You did. Look:
>
>        - description: v2 of TSENS with combined interrupt
> -        enum:
> -          - qcom,ipq8074-tsens
>
> The first character in the diff (-) means removal.

It changed from 'enum' to 'const', that is why I said it is not
removed.

> > For this patch, ipq8074-tsens changed from
> > being an oneof enum entry to a const entry. Probably, that is why
> > dtbs_check is giving these errors.
>
> You removed the entry which you should not have touched.
>
> >
> >>> 	ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> >>> 		['qcom,ipq8074-tsens'] is too short
> >>>
> >>> 	ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
> >>> 		['qcom,ipq8074-tsens'] is too short
> >>>
> >>> I'm not sure of the correct solution. Having the first hunk
> >>> solves the above dtbs_check errors, so went with it. I'm able to
> >>> avoid dtbs_check errors with just one entry in the first hunk.
> >>
> >> You made multiple changes in one patch which is not correct. Your goal
> >> is to add only one change - ipq9574 followed by ipq8074. Add this one.
> >> Don't touch others.
> >
> > But that breaks dtbs_check.
>
> All other cases, hundreds of other binding files, do not have problem.
> Only this one "breaks dtbs_check". No, it does not.
>
> Whatever is broken is result of your removal of unrelated pieces.

Not sure about other binding files. Probably they don't have the
same value for fallback and normal compatible. If there is such
an example binding file, will replicate that syntax/structure for
ipq9574 too.

In the 'nvidia,tegra210-ope' example (https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L25)
too 'nvidia,tegra210-ope' is listed twice

      - const: nvidia,tegra210-ope	<===
      - items:
          - enum:
              - nvidia,tegra234-ope
              - nvidia,tegra194-ope
              - nvidia,tegra186-ope
          - const: nvidia,tegra210-ope	<===

> >>>  	+      - const: qcom,ipq8074-tsens
> >>>
> >>> Please let me know if there is a better way to resolve this or we
> >>> can have just the 8074 entry in the first hunk.
> >>
> >> You only need to add new item on the oneOf list:
> >>  - enum
> >>      - ipq9574
> >>  - const: ipq8074
> >
> > The "['qcom,ipq8074-tsens'] is too short" errors were generated
> > with the above snippet only. Please see the attachment
>
> It's not true. The error you see is result because you removed something
> you should not. I did not ask you to remove anything. So repeating -
> "add new item". Adding is not "removal and adding". Adding is just "adding".

See below for the changes that were tried and the corresponding errors.

(1) No lines removed

	@@ -66,6 +66,7 @@
	       - description: v2 of TSENS with combined interrupt
		 enum:
		   - qcom,ipq8074-tsens
	+          - qcom,ipq9574-tsens

	   reg:
	     items:

	dt_binding_check: No errors

	dtbs_check	:
		arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
	        ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long

(2) No lines removed

	@@ -66,6 +66,8 @@
	       - description: v2 of TSENS with combined interrupt
		 enum:
		   - qcom,ipq8074-tsens
	+          - qcom,ipq9574-tsens
	+        - const: qcom,ipq8074-tsens

	   reg:
	     items:

	dt_binding_check: No errors

	dtbs_check	: Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
		arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
		arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
		arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']


(3) No lines removed
	@@ -19,6 +19,7 @@
	 properties:
	   compatible:
	     oneOf:
	+      - const: qcom,ipq8074-tsens
	       - description: msm8960 TSENS based
		 items:
		   - enum:
	@@ -66,6 +67,8 @@
	       - description: v2 of TSENS with combined interrupt
		 enum:
		   - qcom,ipq8074-tsens
	+          - qcom,ipq9574-tsens
	+        - const: qcom,ipq8074-tsens

	   reg:
	     items:

	dt_binding_check: Same as above

	dtbs_check	: Same as above

(4) Change 8074 from enum to const
	@@ -19,6 +19,7 @@
	 properties:
	   compatible:
	     oneOf:
	+      - const: qcom,ipq8074-tsens
	       - description: msm8960 TSENS based
		 items:
		   - enum:
	@@ -64,8 +65,10 @@
		   - const: qcom,tsens-v2

	       - description: v2 of TSENS with combined interrupt
	-        enum:
	-          - qcom,ipq8074-tsens
	+        items:
	+          - enum:
	+              - qcom,ipq9574-tsens
	+          - const: qcom,ipq8074-tsens

	   reg:
	     items:

	dt_binding_check: No errors

	dtbs_check	: No errors

But (4) doesn't seem acceptable. Any other alternative to resolve this?

Thanks
Varada

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3909 bytes --]

On Tue, May 23, 2023 at 03:49:04PM +0530, Varadarajan Narayanan wrote:
> On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
> > On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> > > On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> > >> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> > >>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> > >>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:

> > It's not true. The error you see is result because you removed something
> > you should not. I did not ask you to remove anything. So repeating -
> > "add new item". Adding is not "removal and adding". Adding is just "adding".
> 
> See below for the changes that were tried and the corresponding errors.
> 
> (1) No lines removed
> 
> 	@@ -66,6 +66,7 @@
> 	       - description: v2 of TSENS with combined interrupt
> 		 enum:
> 		   - qcom,ipq8074-tsens
> 	+          - qcom,ipq9574-tsens
> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: No errors
> 
> 	dtbs_check	:
> 		arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
> 	        ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long

Which I figure you understand makes sense.

> (2) No lines removed
> 
> 	@@ -66,6 +66,8 @@
> 	       - description: v2 of TSENS with combined interrupt
> 		 enum:
> 		   - qcom,ipq8074-tsens
> 	+          - qcom,ipq9574-tsens
> 	+        - const: qcom,ipq8074-tsens
> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: No errors
> 
> 	dtbs_check	: Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
> 		arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
> 		arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
> 		arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']

I think you've missed an earlier error that points out the entire
binding is invalid.

> (3) No lines removed
> 	@@ -19,6 +19,7 @@
> 	 properties:
> 	   compatible:
> 	     oneOf:
> 	+      - const: qcom,ipq8074-tsens
> 	       - description: msm8960 TSENS based
> 		 items:
> 		   - enum:
> 	@@ -66,6 +67,8 @@
> 	       - description: v2 of TSENS with combined interrupt
> 		 enum:
> 		   - qcom,ipq8074-tsens
> 	+          - qcom,ipq9574-tsens
> 	+        - const: qcom,ipq8074-tsens
> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: Same as above
> 
> 	dtbs_check	: Same as above

Ditto here.

> (4) Change 8074 from enum to const
> 	@@ -19,6 +19,7 @@
> 	 properties:
> 	   compatible:
> 	     oneOf:
> 	+      - const: qcom,ipq8074-tsens
> 	       - description: msm8960 TSENS based
> 		 items:
> 		   - enum:
> 	@@ -64,8 +65,10 @@
> 		   - const: qcom,tsens-v2
> 
> 	       - description: v2 of TSENS with combined interrupt
> 	-        enum:
> 	-          - qcom,ipq8074-tsens
> 	+        items:
> 	+          - enum:
> 	+              - qcom,ipq9574-tsens
> 	+          - const: qcom,ipq8074-tsens
> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: No errors
> 
> 	dtbs_check	: No errors
> 
> But (4) doesn't seem acceptable. Any other alternative to resolve this?

It now has a "random" entry up at the top w/ no description, not
matching the existing style. Can you just fix that up & send a v(N+1)
so that the discussion can restart in a less confusing way? I am trying
to fill in for Krzysztof but I am struggling to follow the thread.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Varadarajan Narayanan]

On Tue, May 23, 2023 at 05:44:02PM +0100, Conor Dooley wrote:
> On Tue, May 23, 2023 at 03:49:04PM +0530, Varadarajan Narayanan wrote:
> > On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
> > > On 18/05/2023 11:05, Varadarajan Narayanan wrote:
> > > > On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
> > > >> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
> > > >>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
> > > >>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
> 
> > > It's not true. The error you see is result because you removed something
> > > you should not. I did not ask you to remove anything. So repeating -
> > > "add new item". Adding is not "removal and adding". Adding is just "adding".
> > 
> > See below for the changes that were tried and the corresponding errors.
> > 
> > (1) No lines removed
> > 
> > 	@@ -66,6 +66,7 @@
> > 	       - description: v2 of TSENS with combined interrupt
> > 		 enum:
> > 		   - qcom,ipq8074-tsens
> > 	+          - qcom,ipq9574-tsens
> > 
> > 	   reg:
> > 	     items:
> > 
> > 	dt_binding_check: No errors
> > 
> > 	dtbs_check	:
> > 		arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
> > 	        ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
> 
> Which I figure you understand makes sense.
> 
> > (2) No lines removed
> > 
> > 	@@ -66,6 +66,8 @@
> > 	       - description: v2 of TSENS with combined interrupt
> > 		 enum:
> > 		   - qcom,ipq8074-tsens
> > 	+          - qcom,ipq9574-tsens
> > 	+        - const: qcom,ipq8074-tsens
> > 
> > 	   reg:
> > 	     items:
> > 
> > 	dt_binding_check: No errors
> > 
> > 	dtbs_check	: Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
> > 		arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
> > 		arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
> > 		arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']
> 
> I think you've missed an earlier error that points out the entire
> binding is invalid.
> 
> > (3) No lines removed
> > 	@@ -19,6 +19,7 @@
> > 	 properties:
> > 	   compatible:
> > 	     oneOf:
> > 	+      - const: qcom,ipq8074-tsens
> > 	       - description: msm8960 TSENS based
> > 		 items:
> > 		   - enum:
> > 	@@ -66,6 +67,8 @@
> > 	       - description: v2 of TSENS with combined interrupt
> > 		 enum:
> > 		   - qcom,ipq8074-tsens
> > 	+          - qcom,ipq9574-tsens
> > 	+        - const: qcom,ipq8074-tsens
> > 
> > 	   reg:
> > 	     items:
> > 
> > 	dt_binding_check: Same as above
> > 
> > 	dtbs_check	: Same as above
> 
> Ditto here.
> 
> > (4) Change 8074 from enum to const
> > 	@@ -19,6 +19,7 @@
> > 	 properties:
> > 	   compatible:
> > 	     oneOf:
> > 	+      - const: qcom,ipq8074-tsens
> > 	       - description: msm8960 TSENS based
> > 		 items:
> > 		   - enum:
> > 	@@ -64,8 +65,10 @@
> > 		   - const: qcom,tsens-v2
> > 
> > 	       - description: v2 of TSENS with combined interrupt
> > 	-        enum:
> > 	-          - qcom,ipq8074-tsens
> > 	+        items:
> > 	+          - enum:
> > 	+              - qcom,ipq9574-tsens
> > 	+          - const: qcom,ipq8074-tsens
> > 
> > 	   reg:
> > 	     items:
> > 
> > 	dt_binding_check: No errors
> > 
> > 	dtbs_check	: No errors
> > 
> > But (4) doesn't seem acceptable. Any other alternative to resolve this?
> 
> It now has a "random" entry up at the top w/ no description, not
> matching the existing style. Can you just fix that up & send a v(N+1)
> so that the discussion can restart in a less confusing way? I am trying
> to fill in for Krzysztof but I am struggling to follow the thread.

Sure. Will post the next version.

Thanks
Varada

Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574 compatible

[Krzysztof Kozlowski]

On 23/05/2023 12:19, Varadarajan Narayanan wrote:
> On Thu, May 18, 2023 at 01:06:49PM +0200, Krzysztof Kozlowski wrote:
>> On 18/05/2023 11:05, Varadarajan Narayanan wrote:
>>> On Thu, May 18, 2023 at 09:09:12AM +0200, Krzysztof Kozlowski wrote:
>>>> On 18/05/2023 07:40, Varadarajan Narayanan wrote:
>>>>> On Wed, May 17, 2023 at 09:00:49AM +0200, Krzysztof Kozlowski wrote:
>>>>>> On 17/05/2023 07:57, Varadarajan Narayanan wrote:
>>>>>>> Part-1 is adding the 'const' entries at the beginning i.e.
>>>>>>>
>>>>>>> 	+      - const: qcom,tsens-v0_1
>>>>>>> 	+      - const: qcom,tsens-v1
>>>>>>> 	+      - const: qcom,tsens-v2
>>>>>>> 	+      - const: qcom,ipq8074-tsens
>>>>>>>
>>>>>>> Part-2 is changing from one valid syntax to another i.e.
>>>>>>>
>>>>>>> 	+        items:
>>>>>>> 	+          - enum:
>>>>>>> 	+              - qcom,ipq9574-tsens
>>>>>>> 	+          - const: qcom,ipq8074-tsens
>>>>>>>
>>>>>>> Without both of the above changes, either or both of dtbs_check
>>>>>>> & dt_binding_check fails. So, it is not possible to just add the
>>>>>>> "valid hunk" (part-2) alone.
>>>>>>
>>>>>> Of course it is. All schema files work like that...
>>>>>>>
>>>>>>> If having both part-1 and part-2 in the same patch is not
>>>>>>> acceptable, shall I split them into two patches? Please let me know.
>>>>>>
>>>>>> No, hunk one is not justified.
>>>>>
>>>>> For the other compatibles, the enum entries and const/fallback
>>>>> entries are different. For the 9574 & 8074 case, we want to have
>>>>> qcom,ipq8074-tsens as both enum and const/fallback entry. Hence,
>>>>> if we don't have the first hunk, dtbs_check fails for 8074
>>>>> related dtbs
>>>>>
>>>>> 	ipq8074-hk01.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>>>> 		['qcom,ipq8074-tsens'] is too short
>>>>
>>>> Why? It is already there. Open the file and you will see that this is
>>>> already covered.
>>>
>>> I guess dtbs_check doesn't like the same value being a const and
>>> a oneof entry.
>>
>> I don't understand.
> 
>       - description: v2 of TSENS with combined interrupt
>         items:
>           - enum:
>               - qcom,ipq9574-tsens	<--- one of the compatible entries
>           - const: qcom,ipq8074-tsens	<--- fallback entry
> 
> In this patch, we want 8074 to act as a compatible entry for
> ipq8074*.dts and fallback entry for ipq9574.dtsi. That is why I
> believe we are not able to just add 9574 and get it to pass
> dtbs_check and dt_binding_check.

Nope, no other bindings have any problems with that. Fix your syntax as
I said.

> 
>>>  Have attached the file, please see if something is
>>> not in order.
>>
>> I don't know what changed there. Please work on patches.
>>
>>>
>>>> If you remove it, then yes, you will see errors and the answer is: do
>>>> not remove it.
>>>
>>> I haven't removed it.
>>
>> You did. Look:
>>
>>        - description: v2 of TSENS with combined interrupt
>> -        enum:
>> -          - qcom,ipq8074-tsens
>>
>> The first character in the diff (-) means removal.
> 
> It changed from 'enum' to 'const', that is why I said it is not
> removed.

You removed that hunk. Diff hunk. It does not matter that you added
similar one in different place. You removed this one. Don't.

> 
>>> For this patch, ipq8074-tsens changed from
>>> being an oneof enum entry to a const entry. Probably, that is why
>>> dtbs_check is giving these errors.
>>
>> You removed the entry which you should not have touched.
>>
>>>
>>>>> 	ipq8074-hk10-c2.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>>>> 		['qcom,ipq8074-tsens'] is too short
>>>>>
>>>>> 	ipq8074-hk10-c1.dtb: thermal-sensor@4a9000: compatible: 'oneOf' condition
>>>>> 		['qcom,ipq8074-tsens'] is too short
>>>>>
>>>>> I'm not sure of the correct solution. Having the first hunk
>>>>> solves the above dtbs_check errors, so went with it. I'm able to
>>>>> avoid dtbs_check errors with just one entry in the first hunk.
>>>>
>>>> You made multiple changes in one patch which is not correct. Your goal
>>>> is to add only one change - ipq9574 followed by ipq8074. Add this one.
>>>> Don't touch others.
>>>
>>> But that breaks dtbs_check.
>>
>> All other cases, hundreds of other binding files, do not have problem.
>> Only this one "breaks dtbs_check". No, it does not.
>>
>> Whatever is broken is result of your removal of unrelated pieces.
> 
> Not sure about other binding files. Probably they don't have the
> same value for fallback and normal compatible. If there is such
> an example binding file, will replicate that syntax/structure for
> ipq9574 too.

No. There are many examples of other bindings which do not have any
problems with it and what we talk here is common pattern. You created
some fake problem of wrong syntax and then fixed it with different
approach...

> 
> In the 'nvidia,tegra210-ope' example (https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L25)
> too 'nvidia,tegra210-ope' is listed twice
> 
>       - const: nvidia,tegra210-ope	<===
>       - items:
>           - enum:
>               - nvidia,tegra234-ope
>               - nvidia,tegra194-ope
>               - nvidia,tegra186-ope
>           - const: nvidia,tegra210-ope	<===
> 
>>>>>  	+      - const: qcom,ipq8074-tsens
>>>>>
>>>>> Please let me know if there is a better way to resolve this or we
>>>>> can have just the 8074 entry in the first hunk.
>>>>
>>>> You only need to add new item on the oneOf list:
>>>>  - enum
>>>>      - ipq9574
>>>>  - const: ipq8074
>>>
>>> The "['qcom,ipq8074-tsens'] is too short" errors were generated
>>> with the above snippet only. Please see the attachment
>>
>> It's not true. The error you see is result because you removed something
>> you should not. I did not ask you to remove anything. So repeating -
>> "add new item". Adding is not "removal and adding". Adding is just "adding".
> 
> See below for the changes that were tried and the corresponding errors.
> 
> (1) No lines removed
> 
> 	@@ -66,6 +66,7 @@
> 	       - description: v2 of TSENS with combined interrupt
> 		 enum:
> 		   - qcom,ipq8074-tsens
> 	+          - qcom,ipq9574-tsens

This does not make sense, right? Let's not discuss solutions which do
not make sense...

> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: No errors
> 
> 	dtbs_check	:
> 		arch/arm64/boot/dts/qcom/ipq9574-rdp433.dtb: thermal-sensor@4a9000: compatible: 'oneOf' conditional failed, one must be fixed:
> 	        ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
> 
> (2) No lines removed
> 
> 	@@ -66,6 +66,8 @@
> 	       - description: v2 of TSENS with combined interrupt
> 		 enum:
> 		   - qcom,ipq8074-tsens
> 	+          - qcom,ipq9574-tsens
> 	+        - const: qcom,ipq8074-tsens

You change existing entry, which breaks it. Don't.

> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: No errors
> 
> 	dtbs_check	: Gives errors for all the DTS files that have tsens-v0_1, tsens-v2, tsens-v1. Copy pasted a sample for each one of them
> 		arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8916-tsens', 'qcom,tsens-v0_1']
> 		arch/arm64/boot/dts/qcom/msm8953-xiaomi-tissot.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8953-tsens', 'qcom,tsens-v2']
> 		arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-suzu.dtb: /soc@0/thermal-sensor@4a9000: failed to match any schema with compatible: ['qcom,msm8956-tsens', 'qcom,tsens-v1']
> 
> 
> (3) No lines removed
> 	@@ -19,6 +19,7 @@
> 	 properties:
> 	   compatible:
> 	     oneOf:
> 	+      - const: qcom,ipq8074-tsens
> 	       - description: msm8960 TSENS based
> 		 items:
> 		   - enum:
> 	@@ -66,6 +67,8 @@
> 	       - description: v2 of TSENS with combined interrupt
> 		 enum:
> 		   - qcom,ipq8074-tsens
> 	+          - qcom,ipq9574-tsens
> 	+        - const: qcom,ipq8074-tsens

Don't change existing entry.

> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: Same as above
> 
> 	dtbs_check	: Same as above
> 
> (4) Change 8074 from enum to const
> 	@@ -19,6 +19,7 @@
> 	 properties:
> 	   compatible:
> 	     oneOf:
> 	+      - const: qcom,ipq8074-tsens
> 	       - description: msm8960 TSENS based
> 		 items:
> 		   - enum:
> 	@@ -64,8 +65,10 @@
> 		   - const: qcom,tsens-v2
> 
> 	       - description: v2 of TSENS with combined interrupt
> 	-        enum:
> 	-          - qcom,ipq8074-tsens
> 	+        items:
> 	+          - enum:
> 	+              - qcom,ipq9574-tsens
> 	+          - const: qcom,ipq8074-tsens

Don't touch existing entry. Third time.

> 
> 	   reg:
> 	     items:
> 
> 	dt_binding_check: No errors
> 
> 	dtbs_check	: No errors
> 
> But (4) doesn't seem acceptable. Any other alternative to resolve this?

Don't touch existing entry. Fourth time. Add a new one matching your
combination.


Best regards,
Krzysztof