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

[Conor Dooley <conor@kernel.org>]

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

On Tue, Jul 25, 2023 at 05:28:07AM +0000, Datta, Shubhrajyoti wrote:
> [AMD Official Use Only - General]
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Tuesday, July 25, 2023 12:18 AM
> > To: Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>
> > Cc: devicetree@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>; linux-
> > clk@vger.kernel.org; Simek, Michal <michal.simek@amd.com>;
> > conor+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > robh+dt@kernel.org; sboyd@kernel.org; mturquette@baylibre.com
> > 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@amd.com>
> > >
> > > ---
> > >
> > > 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.

I don't think that you tested it with < 5 clocks (hint, if you remove
one of the clocks from your example below, dt_binding_check should fail).
All the constraints need to move into the `if` bits AFAIU.

Thanks,
Conor.

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