RE: [PATCH v3] dt-bindings: clock: versal: Convert the xlnx,zynqmp-clk.txt to yaml

["Datta, Shubhrajyoti" <shubhrajyoti.datta@xxxxxxx>]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Conor Dooley <conor@xxxxxxxxxx>
> Sent: Tuesday, July 25, 2023 12:18 AM
> To: Datta, Shubhrajyoti <shubhrajyoti.datta@xxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>; linux-
> clk@xxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>;
> conor+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> robh+dt@xxxxxxxxxx; sboyd@xxxxxxxxxx; mturquette@xxxxxxxxxxxx
> Subject: Re: [PATCH v3] dt-bindings: clock: versal: Convert the xlnx,zynqmp-
> clk.txt to yaml
>
> On Mon, Jul 24, 2023 at 04:48:43PM +0530, Shubhrajyoti Datta wrote:
> > Convert the xlnx,zynqmp-clk.txt to yaml.
> > versal-clk.yaml already exists that's why ZynqMP is converted and
> > merged.
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
> >
> > ---
> >
> > Changes in v3:
> > Update the min and maxitems
> >
> > Changes in v2:
> > add enum in compatible
> > fix the description
> > add constraints for clocks
> > name the clock-controller1 to clock-controller
> >
> >  .../bindings/clock/xlnx,versal-clk.yaml       | 78 ++++++++++++++++---
> >  .../bindings/clock/xlnx,zynqmp-clk.txt        | 63 ---------------
> >  2 files changed, 69 insertions(+), 72 deletions(-)  delete mode
> > 100644 Documentation/devicetree/bindings/clock/xlnx,zynqmp-clk.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> > b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> > index e9cf747bf89b..deebbfd084e8 100644
> > --- a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> > +++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> > @@ -19,7 +19,9 @@ select: false
> >  properties:
> >    compatible:
> >      oneOf:
> > -      - const: xlnx,versal-clk
> > +      - enum:
> > +          - xlnx,versal-clk
> > +          - xlnx,zynqmp-clk
> >        - items:
> >            - enum:
> >                - xlnx,versal-net-clk
> > @@ -31,16 +33,12 @@ properties:
> >    clocks:
> >      description: List of clock specifiers which are external input
> >        clocks to the given clock controller.
> > -    items:
> > -      - description: reference clock
> > -      - description: alternate reference clock
> > -      - description: alternate reference clock for programmable logic
> > +    minItems: 3
> > +    maxItems: 7
>
> This doesn't seem right to me. The original binding requires 5 clock inputs,
> but this will relax it such that only three are needed, no?
> You'll need to set constraints on a per compatible basis.
>
Does below look good.

diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
index e9cf747bf89b..89b8d592a6d4 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
@@ -19,7 +19,9 @@ select: false
 properties:
   compatible:
     oneOf:
-      - const: xlnx,versal-clk
+      - enum:
+          - xlnx,versal-clk
+          - xlnx,zynqmp-clk
       - items:
           - enum:
               - xlnx,versal-net-clk
@@ -31,16 +33,12 @@ properties:
   clocks:
     description: List of clock specifiers which are external input
       clocks to the given clock controller.
-    items:
-      - description: reference clock
-      - description: alternate reference clock
-      - description: alternate reference clock for programmable logic
+    minItems: 3
+    maxItems: 7

   clock-names:
-    items:
-      - const: ref
-      - const: alt_ref
-      - const: pl_alt_ref
+    minItems: 3
+    maxItems: 7

 required:
   - compatible
@@ -50,6 +48,61 @@ required:

 additionalProperties: false

+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - xlnx,versal-clk
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: reference clock
+            - description: alternate reference clock
+            - description: alternate reference clock for programmable logic
+
+        clock-names:
+          items:
+            - const: ref
+            - const: alt_ref
+            - const: pl_alt_ref
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - xlnx,zynqmp-clk
+
+    then:
+      properties:
+        clocks:
+          minItems: 5
+          items:
+            - description: PS reference clock
+            - description: reference clock for video system
+            - description: alternative PS reference clock
+            - description: auxiliary reference clock
+            - description: transceiver reference clock
+            - description: (E)MIO clock source  (Optional clock)
+            - description: GEM emio clock  (Optional clock)
+            - description: Watchdog external clock (Optional clock)
+
+        clock-names:
+          minItems: 5
+          items:
+            - const: pss_ref_clk
+            - const: video_clk
+            - const: pss_alt_ref_clk
+            - const: aux_ref_clk
+            - const: gt_crx_ref_clk
+            - pattern: "^mio_clk[00-77]+.*$"
+            - pattern: "gem[0-3]+_emio_clk.*$"
+            - pattern: "swdt[0-1]+_ext_clk.*$"
+
 examples:
   - |
     firmware {
@@ -64,4 +117,13 @@ examples:
         };
       };
     };
+
+    clock-controller {
+        #clock-cells = <1>;
+        compatible = "xlnx,zynqmp-clk";
+        clocks = <&pss_ref_clk>, <&video_clk>, <&pss_alt_ref_clk>,
+                 <&aux_ref_clk>, <&gt_crx_ref_clk>;
+        clock-names = "pss_ref_clk", "video_clk", "pss_alt_ref_clk",
+                      "aux_ref_clk", "gt_crx_ref_clk";
+    };