diff mbox series

[v4,02/18] dt-bindings: clock: qcom: Update sc8280xp camcc bindings

Message ID 20250515-videocc-pll-multi-pd-voting-v4-2-571c63297d01@quicinc.com
State New
Headers show
Series clk: qcom: Add support to attach multiple power domains in cc probe | expand

Commit Message

Jagadeesh Kona May 14, 2025, 7:08 p.m. UTC 
SC8280XP camcc only requires the MMCX power domain, unlike
SM8450 camcc which will now support both MMCX and MXC power
domains. Hence move SC8280XP camcc bindings from SM8450 to
SA8775P camcc.

SA8775P camcc doesn't support required-opps property currently
but SC8280XP camcc need that property,  so add required-opps
based on SC8280XP camcc conditional check in SA8775P camcc
bindings.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 .../devicetree/bindings/clock/qcom,sa8775p-camcc.yaml     | 15 +++++++++++++++
 .../devicetree/bindings/clock/qcom,sm8450-camcc.yaml      |  2 --
 2 files changed, 15 insertions(+), 2 deletions(-)

Comments

Jagadeesh Kona May 21, 2025, 10:02 a.m. UTC | #1 
On 5/19/2025 1:48 PM, Krzysztof Kozlowski wrote:
> On Thu, May 15, 2025 at 12:38:47AM GMT, Jagadeesh Kona wrote:
>> SC8280XP camcc only requires the MMCX power domain, unlike
>> SM8450 camcc which will now support both MMCX and MXC power
> 
> I do not see change to sm8450 here. This makes no sense on its own. You
> do not move compatibles - what is the point of such change?
>

I did the SM8450 changes in next patch (3/18). But I agree with you, this needs to
be more structured. So I am planning to drop this patch and instead take care of
single power domain requirement for SC8280XP within SM8450 camcc bindings using
minItems and maxItems properties based on if check for sc8280xp compatible similar
to below snippet.

   power-domains:
-    maxItems: 1
+    minItems: 1
     description:
-      A phandle and PM domain specifier for the MMCX power domain.
+      Power domains required for the clock controller to operate
+    items:
+      - description: MMCX power domain
+      - description: MXC power domain

......

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc8280xp-camcc
+    then:
+      properties:
+        power-domains:
+          maxItems: 1
+        required-opps:
+          maxItems: 1
+


>> domains. Hence move SC8280XP camcc bindings from SM8450 to
>> SA8775P camcc.
> 
> Subject: everything could be an update. Be specific.
> 
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> 

Sure, I will take care of above in next series.

Thanks,
Jagadeesh

>>
>> SA8775P camcc doesn't support required-opps property currently
>> but SC8280XP camcc need that property,  so add required-opps
>> based on SC8280XP camcc conditional check in SA8775P camcc
>> bindings.
> 
> Best regards,
> Krzysztof
>
Dmitry Baryshkov May 21, 2025, 12:46 p.m. UTC | #2 
On Wed, May 21, 2025 at 03:32:34PM +0530, Jagadeesh Kona wrote:
> 
> 
> On 5/19/2025 1:48 PM, Krzysztof Kozlowski wrote:
> > On Thu, May 15, 2025 at 12:38:47AM GMT, Jagadeesh Kona wrote:
> >> SC8280XP camcc only requires the MMCX power domain, unlike
> >> SM8450 camcc which will now support both MMCX and MXC power
> > 
> > I do not see change to sm8450 here. This makes no sense on its own. You
> > do not move compatibles - what is the point of such change?
> >
> 
> I did the SM8450 changes in next patch (3/18). But I agree with you, this needs to
> be more structured. So I am planning to drop this patch and instead take care of
> single power domain requirement for SC8280XP within SM8450 camcc bindings using
> minItems and maxItems properties based on if check for sc8280xp compatible similar
> to below snippet.

I think it is a bad idea. I liked the split that you've implemented:
separate bindings for platforms that require MMCX (and MX), separate
bindings for platforms which require MMCX and MXC (and MXA).

It might be better to start by changing SM8450 binding to support MXC
and then adding SC8280XP to those bindings.

> 
>    power-domains:
> -    maxItems: 1
> +    minItems: 1
>      description:
> -      A phandle and PM domain specifier for the MMCX power domain.
> +      Power domains required for the clock controller to operate
> +    items:
> +      - description: MMCX power domain
> +      - description: MXC power domain
> 
> ......
> 
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sc8280xp-camcc
> +    then:
> +      properties:
> +        power-domains:
> +          maxItems: 1
> +        required-opps:
> +          maxItems: 1
> +
> 
> 
> >> domains. Hence move SC8280XP camcc bindings from SM8450 to
> >> SA8775P camcc.
> > 
> > Subject: everything could be an update. Be specific.
> > 
> > A nit, subject: drop second/last, redundant "bindings". The
> > "dt-bindings" prefix is already stating that these are bindings.
> > See also:
> > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> > 
> 
> Sure, I will take care of above in next series.
> 
> Thanks,
> Jagadeesh
> 
> >>
> >> SA8775P camcc doesn't support required-opps property currently
> >> but SC8280XP camcc need that property,  so add required-opps
> >> based on SC8280XP camcc conditional check in SA8775P camcc
> >> bindings.
> > 
> > Best regards,
> > Krzysztof
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
index 81623f59d11d73839e5c551411a52427e2f28415..f42ccb6627a387ee0d0238ebd1fcd1cdf64c5676 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sa8775p-camcc.yaml
@@ -17,12 +17,14 @@  description: |
   See also:
     include/dt-bindings/clock/qcom,qcs8300-camcc.h
     include/dt-bindings/clock/qcom,sa8775p-camcc.h
+    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
 
 properties:
   compatible:
     enum:
       - qcom,qcs8300-camcc
       - qcom,sa8775p-camcc
+      - qcom,sc8280xp-camcc
 
   clocks:
     items:
@@ -35,6 +37,11 @@  properties:
     maxItems: 1
     description: MMCX power domain
 
+  required-opps:
+    description:
+      OPP node describing required MMCX performance point.
+    maxItems: 1
+
 required:
   - compatible
   - clocks
@@ -43,6 +50,14 @@  required:
 
 allOf:
   - $ref: qcom,gcc.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,sc8280xp-camcc
+    then:
+      required:
+        - required-opps
 
 unevaluatedProperties: false
 
diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
index 9e79f8fec437b9aecb5103092f6ff2ad1cd42626..883f12e3d11fa16384108434f6de120162226a28 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
@@ -15,7 +15,6 @@  description: |
   domains on SM8450.
 
   See also:
-    include/dt-bindings/clock/qcom,sc8280xp-camcc.h
     include/dt-bindings/clock/qcom,sm8450-camcc.h
     include/dt-bindings/clock/qcom,sm8550-camcc.h
     include/dt-bindings/clock/qcom,sm8650-camcc.h
@@ -23,7 +22,6 @@  description: |
 properties:
   compatible:
     enum:
-      - qcom,sc8280xp-camcc
       - qcom,sm8450-camcc
       - qcom,sm8475-camcc
       - qcom,sm8550-camcc