On 18/08/2024 19:50, Laurent Pinchart wrote: > 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. First, they apply to ALL variants using clock-names. Second, we want such lists, like clocks/resets/interrupts, to share as much as possible between variants, e.g. keep the same order. Having clock-names listed at top-level encourages this and prevents people from adding new binding with: "vclk", "aclk", "pclk", "new_clock_but_i_want_to_mess_order_of_everything_because_i_can" > >>>> >>>> - 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. Could be main clock, could be something else for me - I did not investigate enough. If it is main clock, I will move the description as well. Best regards, Krzysztof
