Re: [PATCH v5 1/7] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Icenowy Zheng]

在 2026-06-26五的 08:19 +0100，Conor Dooley写道：
> On Fri, Jun 26, 2026 at 01:27:21PM +0800, Icenowy Zheng wrote:
> > 在 2026-06-25四的 17:33 +0100，Conor Dooley写道：
> > > On Thu, Jun 25, 2026 at 05:44:43PM +0800, Joey Lu wrote:
> > > > +allOf:
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: thead,th1520-dc8200
> > > > +    then:
> > > > +      properties:
> > > > +        clocks:
> > > > +          minItems: 5
> > > > +          maxItems: 5
> > > > +
> > > > +        clock-names:
> > > > +          minItems: 5
> > > > +          maxItems: 5
> > > 
> > > All the maxItems here repeat the maximum constraint and do
> > > nothing.
> > > 
> > > Since you didn't change the minimum constraint at the top level,
> > > your
> > > minItems also do nothing.
> > > 
> > > > +
> > > > +        resets:
> > > > +          minItems: 3
> > > > +          maxItems: 3
> > > > +
> > > > +        reset-names:
> > > > +          minItems: 3
> > > > +          maxItems: 3
> > > > +
> > > > +      required:
> > > > +        - resets
> > > > +        - reset-names
> > > 
> > > Both conditional sections have this, but the original binding
> > > doesn't
> > > require these for the thead device. This is a functional change
> > > therefore and shouldn't be in a patch calling itself "generalise
> > > for
> > > single ended variants".
> > 
> > Well yes they're required.
> > 
> > Should I send a patch adding the `thead,th1520-dc8200` part of the
> > schema?
> 
> If you mean the code above, no. Adding a conditional section when
> there's only that compatible doesn't make sense.
> 
> What you could do is just add it at the top level though, which would
> also benefit this patch since it'd not have to be conditionally added
> for the new nuvoton device.
> Just note in your commit message about what the ABI impact of the
> change
> to required properties is (effectively nothing because it's optional
> in
> the driver and the only user has the properties).

Okay, I will craft such a patch and send it.

> 
> > > > +
> > > > +        resets:
> > > > +          minItems: 1
> > > > +          maxItems: 1
> > > > +
> > > > +        reset-names:
> > > > +          items:
> > > > +            - const: core
> > > 
> > > This is just maxItems: 1.
> > 
> > Well the implicit rules of DT binding schemas are quite weird...
> 
> I don't think it is that strange, as the binding has
>   reset-names:
>     items:
>       - const: core
>       - const: axi
>       - const: ahb

Ah does the list constraint the order of items? If it constrains the
order, it partly breaks the intention of having names; if it does not
constrain the order, it needs to be clarified that the required 1 reset
is core instead of the other two.

Thanks,
Icenowy

> so just constraining to one item is the simplest way to do this
> without
> duplication.