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

[Conor Dooley <conor.dooley@microchip.com>]

[-- Attachment #1: Type: text/plain, Size: 2657 bytes --]

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).

> > > +
> > > +        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
so just constraining to one item is the simplest way to do this without
duplication.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]