On 28/07/2023 18:19, Conor Dooley wrote: > On Fri, Jul 28, 2023 at 06:41:50AM +0000, Datta, Shubhrajyoti wrote: >> [AMD Official Use Only - General] >> >>> -----Original Message----- >>> From: Conor Dooley <conor@xxxxxxxxxx> >>> Sent: Wednesday, July 26, 2023 12:57 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 Tue, Jul 25, 2023 at 05:28:07AM +0000, Datta, Shubhrajyoti wrote: >>>> [AMD Official Use Only - General] >>>> >> >> <snip> >>>>>> 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. >> >> >> https://lore.kernel.org/all/20230720113110.25047-1-shubhrajyoti.datta@xxxxxxx/ >> Here I had it in the if . >> Then what I understood from below is that >> >> https://lore.kernel.org/all/745fccb0-e49d-7da7-9556-eb28aee4a32b@xxxxxxxxxx/ >> it should be dropped from the if and added to the above. >> >> Maybe I am missing something. > > (Background I got this mail once off-list and tried to make the > binding's validation work) > > With the current conditions, validation is completely broken. You can > put in just 1 clock and 1 clock-name and dt-binding-check will pass. The > only way I could satisfy it, I don't understand why you think 1 clock would work. The binding has clear min/max constraints in top level and strict constraints per each variant (through listing items). In my opinion this is correct, except what I wrote - mismatched number of clocks for zynqmp (8 against 7). > while keeping 7 as the maximum number of > clocks, was moving the constraints into the if/else. My guess was that 7 > being fewer than the number of clocks in the items: list is part of the > problem. Best regards, Krzysztof
