[PATCH v3 0/3] arm: qcom: qcom-apq8064: add separate device node for tsens

[PATCH v3 0/3] arm: qcom: qcom-apq8064: add separate device node for tsens

[Dmitry Baryshkov]

Currently gcc-msm8960 driver manually creates tsens device. Instantiate
the device using DT node instead. This makes the APQ8064 follow the
IPQ8064 device tree schema (which is also closer to the way tsens
devices are described on newer Qualcomm platforms).

Compatibility with the previous devices trees is kept intact.

Changes since v2:
- Remove patches merged by Daniel
- Rephrase commit messages to emphasize that these changes make apq8064
  follow ipq8064

Changes since v1:
- populate child devices in gcc-msm8960
- add syscon to the gcc device tree node

Dmitry Baryshkov (3):
  clk: qcom: gcc-msm8960: create tsens device if there are no child
    nodes
  arm: dts: qcom-apq8064: create tsens device node
  dt-bindings: clock: qcom,gcc-apq8064: split tsens to the child node

 .../bindings/clock/qcom,gcc-apq8064.yaml      | 45 +++++++------------
 arch/arm/boot/dts/qcom-apq8064.dtsi           | 25 +++++++----
 drivers/clk/qcom/gcc-msm8960.c                |  6 ++-
 3 files changed, 39 insertions(+), 37 deletions(-)

-- 
2.35.1

[PATCH v3 1/3] clk: qcom: gcc-msm8960: create tsens device if there are no child nodes

[Dmitry Baryshkov]

Currently gcc-msm8960 driver manually creates tsens platform device
manually. It would be better to follow IPQ8064 approach, where tsens
device is defined as gcc's child device in the device tree. If nothing
else, it removes gcc's dependency on QFPROM, thus allowing clock
controller to be probed earlier.

Don't create it in case there are available child nodes (tsens) inside
the gcc's device tree node.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/gcc-msm8960.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-msm8960.c b/drivers/clk/qcom/gcc-msm8960.c
index 051745ef99c8..a6e13b91e4c8 100644
--- a/drivers/clk/qcom/gcc-msm8960.c
+++ b/drivers/clk/qcom/gcc-msm8960.c
@@ -3641,6 +3641,9 @@ static int gcc_msm8960_probe(struct platform_device *pdev)
 		hfpll_l2.d = &hfpll_l2_8064_data;
 	}
 
+	if (of_get_available_child_count(pdev->dev.of_node) != 0)
+		return devm_of_platform_populate(&pdev->dev);
+
 	tsens = platform_device_register_data(&pdev->dev, "qcom-tsens", -1,
 					      NULL, 0);
 	if (IS_ERR(tsens))
@@ -3655,7 +3658,8 @@ static int gcc_msm8960_remove(struct platform_device *pdev)
 {
 	struct platform_device *tsens = platform_get_drvdata(pdev);
 
-	platform_device_unregister(tsens);
+	if (tsens)
+		platform_device_unregister(tsens);
 
 	return 0;
 }
-- 
2.35.1

[PATCH v3 2/3] arm: dts: qcom-apq8064: create tsens device node

[Dmitry Baryshkov]

Create separate device node for thermal sensors on apq8064 platform.
Move related properties to the newly created device tree node.
This harmonizes apq8064 and ipq8064 device trees and allows gcc device
to be probed earlier by removing dependency on QFPROM nodes.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8064.dtsi | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index a1c8ae516d21..389191ca5a69 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -105,7 +105,7 @@ cpu0-thermal {
 			polling-delay-passive = <250>;
 			polling-delay = <1000>;
 
-			thermal-sensors = <&gcc 7>;
+			thermal-sensors = <&tsens 7>;
 			coefficients = <1199 0>;
 
 			trips {
@@ -126,7 +126,7 @@ cpu1-thermal {
 			polling-delay-passive = <250>;
 			polling-delay = <1000>;
 
-			thermal-sensors = <&gcc 8>;
+			thermal-sensors = <&tsens 8>;
 			coefficients = <1132 0>;
 
 			trips {
@@ -147,7 +147,7 @@ cpu2-thermal {
 			polling-delay-passive = <250>;
 			polling-delay = <1000>;
 
-			thermal-sensors = <&gcc 9>;
+			thermal-sensors = <&tsens 9>;
 			coefficients = <1199 0>;
 
 			trips {
@@ -168,7 +168,7 @@ cpu3-thermal {
 			polling-delay-passive = <250>;
 			polling-delay = <1000>;
 
-			thermal-sensors = <&gcc 10>;
+			thermal-sensors = <&tsens 10>;
 			coefficients = <1132 0>;
 
 			trips {
@@ -810,14 +810,23 @@ tsens_backup: backup_calib {
 		};
 
 		gcc: clock-controller@900000 {
-			compatible = "qcom,gcc-apq8064";
+			compatible = "qcom,gcc-apq8064", "syscon";
 			reg = <0x00900000 0x4000>;
-			nvmem-cells = <&tsens_calib>, <&tsens_backup>;
-			nvmem-cell-names = "calib", "calib_backup";
 			#clock-cells = <1>;
 			#power-domain-cells = <1>;
 			#reset-cells = <1>;
-			#thermal-sensor-cells = <1>;
+
+			tsens: thermal-sensor {
+				compatible = "qcom,msm8960-tsens";
+
+				nvmem-cells = <&tsens_calib>, <&tsens_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>;
+			};
 		};
 
 		lcc: clock-controller@28000000 {
-- 
2.35.1

[PATCH v3 3/3] dt-bindings: clock: qcom,gcc-apq8064: split tsens to the child node

[Dmitry Baryshkov]

Split tsens properties to the child node of the gcc. This follows the
lead of ipq8064 (which also uses a separate node for tsens) and makes
device tree closer to other platforms, where tsens is a completely
separate device.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../bindings/clock/qcom,gcc-apq8064.yaml      | 45 +++++++------------
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
index b867da12761e..f2762599f679 100644
--- a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
@@ -23,47 +23,36 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - qcom,gcc-apq8064
-      - qcom,gcc-msm8060
-
-  nvmem-cells:
-    minItems: 1
-    maxItems: 2
-    description:
-      Qualcomm TSENS (thermal sensor device) on some devices can
-      be part of GCC and hence the TSENS properties can also be part
-      of the GCC/clock-controller node.
-      For more details on the TSENS properties please refer
-      Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
-
-  nvmem-cell-names:
-    minItems: 1
     items:
-      - const: calib
-      - const: calib_backup
-
-  '#thermal-sensor-cells':
-    const: 1
+      - enum:
+          - qcom,gcc-apq8064
+          - qcom,gcc-msm8060
+      - const: syscon
 
 required:
   - compatible
-  - nvmem-cells
-  - nvmem-cell-names
-  - '#thermal-sensor-cells'
 
 unevaluatedProperties: false
 
 examples:
   - |
     clock-controller@900000 {
-      compatible = "qcom,gcc-apq8064";
+      compatible = "qcom,gcc-apq8064", "syscon";
       reg = <0x00900000 0x4000>;
-      nvmem-cells = <&tsens_calib>, <&tsens_backup>;
-      nvmem-cell-names = "calib", "calib_backup";
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;
-      #thermal-sensor-cells = <1>;
+
+      thermal-sensor {
+        compatible = "qcom,msm8960-tsens";
+
+        nvmem-cells = <&tsens_calib>, <&tsens_backup>;
+        nvmem-cell-names = "calib", "calib_backup";
+        interrupts = <0 178 4>;
+        interrupt-names = "uplow";
+
+        #qcom,sensors = <11>;
+        #thermal-sensor-cells = <1>;
+        };
     };
 ...
-- 
2.35.1

Re: [PATCH v3 3/3] dt-bindings: clock: qcom,gcc-apq8064: split tsens to the child node

[Krzysztof Kozlowski]

On 25/04/2022 23:27, Dmitry Baryshkov wrote:
> Split tsens properties to the child node of the gcc. This follows the
> lead of ipq8064 (which also uses a separate node for tsens) and makes
> device tree closer to other platforms, where tsens is a completely
> separate device.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../bindings/clock/qcom,gcc-apq8064.yaml      | 45 +++++++------------
>  1 file changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> index b867da12761e..f2762599f679 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> @@ -23,47 +23,36 @@ description: |
>  
>  properties:
>    compatible:
> -    enum:
> -      - qcom,gcc-apq8064
> -      - qcom,gcc-msm8060

Hm, such list does not exist in mainline, so is your set rebased on
something? See also:
https://lore.kernel.org/linux-devicetree/20220425133527.3723233-1-robh@kernel.org/
https://lore.kernel.org/linux-devicetree/20220426064241.6379-1-krzysztof.kozlowski@linaro.org/


> -
> -  nvmem-cells:
> -    minItems: 1
> -    maxItems: 2
> -    description:
> -      Qualcomm TSENS (thermal sensor device) on some devices can
> -      be part of GCC and hence the TSENS properties can also be part
> -      of the GCC/clock-controller node.
> -      For more details on the TSENS properties please refer
> -      Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> -
> -  nvmem-cell-names:
> -    minItems: 1
>      items:
> -      - const: calib
> -      - const: calib_backup

I see the removal of tsens properties, but I do not see the other part
of split - adding them as child. It does not come from qcom,gcc.yaml,
either.

> -
> -  '#thermal-sensor-cells':
> -    const: 1
> +      - enum:
> +          - qcom,gcc-apq8064
> +          - qcom,gcc-msm8060
> +      - const: syscon
>  
>  required:
>    - compatible
> -  - nvmem-cells
> -  - nvmem-cell-names
> -  - '#thermal-sensor-cells'
>  
>  unevaluatedProperties: false
>  
>  examples:
>    - |
>      clock-controller@900000 {
> -      compatible = "qcom,gcc-apq8064";
> +      compatible = "qcom,gcc-apq8064", "syscon";
>        reg = <0x00900000 0x4000>;
> -      nvmem-cells = <&tsens_calib>, <&tsens_backup>;
> -      nvmem-cell-names = "calib", "calib_backup";
>        #clock-cells = <1>;
>        #reset-cells = <1>;
>        #power-domain-cells = <1>;
> -      #thermal-sensor-cells = <1>;
> +
> +      thermal-sensor {
> +        compatible = "qcom,msm8960-tsens";
> +
> +        nvmem-cells = <&tsens_calib>, <&tsens_backup>;
> +        nvmem-cell-names = "calib", "calib_backup";
> +        interrupts = <0 178 4>;

0 and 4 look like GIC/interrupt flags, so please use defines.

> +        interrupt-names = "uplow";
> +
> +        #qcom,sensors = <11>;
> +        #thermal-sensor-cells = <1>;
> +        };

Indentation looks weird here.

>      };
>  ...


Best regards,
Krzysztof

Re: [PATCH v3 3/3] dt-bindings: clock: qcom,gcc-apq8064: split tsens to the child node

[Dmitry Baryshkov]

On 26/04/2022 10:08, Krzysztof Kozlowski wrote:
> On 25/04/2022 23:27, Dmitry Baryshkov wrote:
>> Split tsens properties to the child node of the gcc. This follows the
>> lead of ipq8064 (which also uses a separate node for tsens) and makes
>> device tree closer to other platforms, where tsens is a completely
>> separate device.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   .../bindings/clock/qcom,gcc-apq8064.yaml      | 45 +++++++------------
>>   1 file changed, 17 insertions(+), 28 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
>> index b867da12761e..f2762599f679 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
>> @@ -23,47 +23,36 @@ description: |
>>   
>>   properties:
>>     compatible:
>> -    enum:
>> -      - qcom,gcc-apq8064
>> -      - qcom,gcc-msm8060
> 
> Hm, such list does not exist in mainline, so is your set rebased on
> something? See also:
> https://lore.kernel.org/linux-devicetree/20220425133527.3723233-1-robh@kernel.org/
> https://lore.kernel.org/linux-devicetree/20220426064241.6379-1-krzysztof.kozlowski@linaro.org/

Ugh, yes. I missed a patch during git send-email. I've sent it 
separately (dt-bindings: clock: gcc-apq8064: move qcom,apq8084 back to 
gcc-other.yaml)

> 
> 
>> -
>> -  nvmem-cells:
>> -    minItems: 1
>> -    maxItems: 2
>> -    description:
>> -      Qualcomm TSENS (thermal sensor device) on some devices can
>> -      be part of GCC and hence the TSENS properties can also be part
>> -      of the GCC/clock-controller node.
>> -      For more details on the TSENS properties please refer
>> -      Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>> -
>> -  nvmem-cell-names:
>> -    minItems: 1
>>       items:
>> -      - const: calib
>> -      - const: calib_backup
> 
> I see the removal of tsens properties, but I do not see the other part
> of split - adding them as child. It does not come from qcom,gcc.yaml,
> either.
> 
>> -
>> -  '#thermal-sensor-cells':
>> -    const: 1
>> +      - enum:
>> +          - qcom,gcc-apq8064
>> +          - qcom,gcc-msm8060
>> +      - const: syscon
>>   
>>   required:
>>     - compatible
>> -  - nvmem-cells
>> -  - nvmem-cell-names
>> -  - '#thermal-sensor-cells'
>>   
>>   unevaluatedProperties: false
>>   
>>   examples:
>>     - |
>>       clock-controller@900000 {
>> -      compatible = "qcom,gcc-apq8064";
>> +      compatible = "qcom,gcc-apq8064", "syscon";
>>         reg = <0x00900000 0x4000>;
>> -      nvmem-cells = <&tsens_calib>, <&tsens_backup>;
>> -      nvmem-cell-names = "calib", "calib_backup";
>>         #clock-cells = <1>;
>>         #reset-cells = <1>;
>>         #power-domain-cells = <1>;
>> -      #thermal-sensor-cells = <1>;
>> +
>> +      thermal-sensor {
>> +        compatible = "qcom,msm8960-tsens";
>> +
>> +        nvmem-cells = <&tsens_calib>, <&tsens_backup>;
>> +        nvmem-cell-names = "calib", "calib_backup";
>> +        interrupts = <0 178 4>;
> 
> 0 and 4 look like GIC/interrupt flags, so please use defines.
> 
>> +        interrupt-names = "uplow";
>> +
>> +        #qcom,sensors = <11>;
>> +        #thermal-sensor-cells = <1>;
>> +        };
> 
> Indentation looks weird here.
> 
>>       };
>>   ...
> 
> 
> Best regards,
> Krzysztof


-- 
With best wishes
Dmitry