On 28/07/2023 18:47, Conor Dooley wrote: > On Fri, Jul 28, 2023 at 06:39:23PM +0200, Krzysztof Kozlowski wrote: >> 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). > > I didn't expect it would work - I tried 4 w/ the zynqmp compatible, > which passed, although it should not have. And then I took it to the > extreme (1) which also passed. There's something wrong with either the > binding or the example that I can't spot. > Yep, select: false. This is some junk code :( Best regards, Krzysztof
