diff mbox series

[1/8] dt-bindings: clock: qcom,sm7150-gcc: Add missing CX power domain

Message ID 20240220165240.154716-2-danila@jiaxyga.com
State New
Headers
Series Add dispcc, videocc and camcc for SM7150. |

Commit Message

Danila Tikhonov Feb. 20, 2024, 4:52 p.m. UTC 
  SM7150 GCC expected two power domains - CX and CX_AO. Currently only
one is supported, so add the missing CX.

Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
---
 .../devicetree/bindings/clock/qcom,sm7150-gcc.yaml        | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Rob Herring Feb. 23, 2024, 1:43 p.m. UTC | #1 
On Tue, Feb 20, 2024 at 07:52:33PM +0300, Danila Tikhonov wrote:
> SM7150 GCC expected two power domains - CX and CX_AO. Currently only
> one is supported, so add the missing CX.

This makes no sense. You had 0 and now you have 1 power domain, not 2. 
Where is CX_AO.

> 
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
>  .../devicetree/bindings/clock/qcom,sm7150-gcc.yaml        | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
> index 0eb76d9d51c4..1360e9d1d6ee 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
> @@ -27,9 +27,15 @@ properties:
>        - description: Board XO Active-Only source
>        - description: Sleep clock source
>  
> +  power-domains:
> +    maxItems: 1
> +    description:
> +      CX power domain.
> +
>  required:
>    - compatible
>    - clocks
> +  - power-domains

Adding new required properties is an ABI break. If that is fine, you 
must say why in the commit message.

>  
>  allOf:
>    - $ref: qcom,gcc.yaml#
> @@ -39,12 +45,14 @@ unevaluatedProperties: false
>  examples:
>    - |
>      #include <dt-bindings/clock/qcom,rpmh.h>
> +    #include <dt-bindings/power/qcom,rpmhpd.h>
>      clock-controller@100000 {
>        compatible = "qcom,sm7150-gcc";
>        reg = <0x00100000 0x001f0000>;
>        clocks = <&rpmhcc RPMH_CXO_CLK>,
>                 <&rpmhcc RPMH_CXO_CLK_A>,
>                 <&sleep_clk>;
> +      power-domains = <&rpmhpd RPMHPD_CX>;
>        #clock-cells = <1>;
>        #reset-cells = <1>;
>        #power-domain-cells = <1>;
> -- 
> 2.43.2
>
Danila Tikhonov Feb. 24, 2024, 5:39 p.m. UTC | #2 
On 2/23/24 16:43, Rob Herring wrote:
> On Tue, Feb 20, 2024 at 07:52:33PM +0300, Danila Tikhonov wrote:
>> SM7150 GCC expected two power domains - CX and CX_AO. Currently only
>> one is supported, so add the missing CX.
> This makes no sense. You had 0 and now you have 1 power domain, not 2.
> Where is CX_AO.
>
Konrad Dybcio and Dmitry Baryshkov informed me on IRC that currently
only one power-domain is supported. Otherwise both will be ignored. If
we add both, it will cause confusion.
>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
>> ---
>>   .../devicetree/bindings/clock/qcom,sm7150-gcc.yaml        | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
>> index 0eb76d9d51c4..1360e9d1d6ee 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
>> @@ -27,9 +27,15 @@ properties:
>>         - description: Board XO Active-Only source
>>         - description: Sleep clock source
>>   
>> +  power-domains:
>> +    maxItems: 1
>> +    description:
>> +      CX power domain.
>> +
>>   required:
>>     - compatible
>>     - clocks
>> +  - power-domains
> Adding new required properties is an ABI break. If that is fine, you
> must say why in the commit message.
I think everything is fine. The DTs for the SM7150 are not yet upstream.
We have been using this parameter for a very long time in our community
fork. So there should be no problems.
I am not sure what else could be added to the commit message. The loss
of power domains was simply my fault.
I think waiting for the support of two power domains is a good solution.
However, for now, I can simply drop this patch for next version.

---
Best wishes
Danila
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
index 0eb76d9d51c4..1360e9d1d6ee 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm7150-gcc.yaml
@@ -27,9 +27,15 @@  properties:
       - description: Board XO Active-Only source
       - description: Sleep clock source
 
+  power-domains:
+    maxItems: 1
+    description:
+      CX power domain.
+
 required:
   - compatible
   - clocks
+  - power-domains
 
 allOf:
   - $ref: qcom,gcc.yaml#
@@ -39,12 +45,14 @@  unevaluatedProperties: false
 examples:
   - |
     #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/power/qcom,rpmhpd.h>
     clock-controller@100000 {
       compatible = "qcom,sm7150-gcc";
       reg = <0x00100000 0x001f0000>;
       clocks = <&rpmhcc RPMH_CXO_CLK>,
                <&rpmhcc RPMH_CXO_CLK_A>,
                <&sleep_clk>;
+      power-domains = <&rpmhpd RPMHPD_CX>;
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;