Re: [PATCH 1/2] media: dt-bindings: renesas,fcp: add top-level constraints

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Sun, Aug 18, 2024 at 07:45:55PM +0200, Krzysztof Kozlowski wrote:
> On 18/08/2024 19:37, Laurent Pinchart wrote:
> > On Sun, Aug 18, 2024 at 07:29:36PM +0200, Krzysztof Kozlowski wrote:
> >> Properties with variable number of items per each device are expected to
> >> have widest constraints in top-level "properties:" block and further
> >> customized (narrowed) in "if:then:".  Add missing top-level constraints
> >> for clocks and clock-names.
> > 
> > In this specific case I think it's fine, but generally speaking, how do
> > you handle that rule when different variants have completely different
> > clocks, not just lack some of the clocks ?
> 
> I don't understand the problem. We handle it as usual, as in all
> bindings. Here there is no such case, thus names go to the top.

That answers the question, the clock names would still be
variant-specific in that case.

While the change here won't cause validation failures, I think it's
confusing to define the clock names at the top level, knowing they don't
apply to some of the variants, if we don't also define the description
there. I'd move either both or neither.

> >>  
> >> -  clock-names: true
> >> +  clock-names:
> >> +    items:
> >> +      - const: aclk
> >> +      - const: pclk
> >> +      - const: vclk
> >>  
> >>    iommus:
> >>      maxItems: 1
> >> @@ -71,11 +77,6 @@ allOf:
> >>              - description: Main clock
> >>              - description: Register access clock
> >>              - description: Video clock
> >> -        clock-names:
> >> -          items:
> >> -            - const: aclk
> >> -            - const: pclk
> >> -            - const: vclk
> > 
> > Any specific reason to move the clock names but not the descriptions ?
> > The assymetry bothers me.
> 
> The other variant does not have description of the first clock, so
> moving it would be incorrect. Moving names is correct, because other
> variant does not have clock-names at all.

I don't think it's incorrect, when the FCP has a single clock, it's the
main clock.

-- 
Regards,

Laurent Pinchart