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. > > 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 >
Attachment:
signature.asc
Description: PGP signature
