On Tue, May 19, 2026 at 03:26:58PM +0800, Icenowy Zheng wrote: > 在 2026-05-19二的 13:51 +0800，Joey Lu写道： > > The existing schema assumes a fixed clock/reset topology and dual- > > output > > port structure matching the DC8200 IP block. This prevents reuse for > > single-output variants such as the Verisilicon DCU Lite used in the > > Nuvoton MA35D1 SoC. > > > > Rework the schema so that variant-specific constraints are expressed > > via allOf/if-then-else: > > > > - The thead,th1520-dc8200 compatible keeps its existing five-clock, > > three-reset, dual-port requirements. > > > > - A standalone verisilicon,dc compatible covers IPs whose identity is > > discovered entirely through hardware registers; these have flexible > > clock and reset counts, a single 'port' property, and no 'ports' > > requirement. > > > > Changes to the base schema: > > - Replace the fixed clock/reset items lists with minItems/maxItems > > ranges; variant sub-schemas tighten the constraints via if-then- > > else. > > - Add a 'port' property (graph.yaml single-port alias) alongside the > > existing 'ports', for single-output variants. > > - Drop the unconditional 'ports' requirement; each if-branch enforces > > its own port topology. > > - Tighten additionalProperties to unevaluatedProperties to allow > > per-variant schemas to add their own constraints cleanly. > > - Fix a stray space in the port@0 description. > > - Add a DT example for the generic verisilicon,dc compatible > > (Nuvoton MA35D1 DCU Lite). > > > > Signed-off-by: Joey Lu <[email protected]> > > --- > > .../bindings/display/verisilicon,dc.yaml | 135 ++++++++++++++-- > > -- > > 1 file changed, 108 insertions(+), 27 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml > > b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml > > index 9dc35ab973f2..3a814c2e083e 100644 > > --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml > > +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml > > @@ -14,10 +14,12 @@ properties: > > pattern: "^display@[0-9a-f]+$" > > > > compatible: > > - items: > > - - enum: > > - - thead,th1520-dc8200 > > You should add a fallback compatible here for your SoC, in case its > integration gets something quirky; this compatible is usually not > consumed by the driver (see how thead,th1520-dc8200 exists in the > binding but not the driver).
s/fallback compatible/soc-specific compatible/, but yes.
NAK to what's been done here, especially after the discussions on
earlier versions of this verisilicon binding.
pw-bot: changes-requested
>
> > - - const: verisilicon,dc # DC IPs have discoverable ID/revision
> > registers
> > + oneOf:
> > + - items:
> > + - enum:
> > + - thead,th1520-dc8200
> > + - const: verisilicon,dc
> > + - const: verisilicon,dc # DC IPs have discoverable
> > ID/revision registers
> >
> > reg:
> > maxItems: 1
> > @@ -26,32 +28,24 @@ properties:
> > maxItems: 1
> >
> > clocks:
> > - items:
> > - - description: DC Core clock
> > - - description: DMA AXI bus clock
> > - - description: Configuration AHB bus clock
> > - - description: Pixel clock of output 0
> > - - description: Pixel clock of output 1
> > + minItems: 2
> > + maxItems: 5
> >
> > clock-names:
> > - items:
> > - - const: core
> > - - const: axi
> > - - const: ahb
> > - - const: pix0
> > - - const: pix1
> > + minItems: 2
> > + maxItems: 5
> >
> > resets:
> > - items:
> > - - description: DC Core reset
> > - - description: DMA AXI bus reset
> > - - description: Configuration AHB bus reset
> > + minItems: 1
> > + maxItems: 3
> >
> > reset-names:
> > - items:
> > - - const: core
> > - - const: axi
> > - - const: ahb
> > + minItems: 1
> > + maxItems: 3
> > +
> > + port:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: Single video output port for single-output
> > variants.
>
> Maybe the endpoint numbering rule needs a move to here? (I am not very
> sure).
>
> >
> > ports:
> > $ref: /schemas/graph.yaml#/properties/ports
> > @@ -59,7 +53,7 @@ properties:
> > properties:
> > port@0:
> > $ref: /schemas/graph.yaml#/properties/port
> > - description: The first output channel , endpoint 0 should be
> > + description: The first output channel, endpoint 0 should be
> > used for DPI format output and endpoint 1 should be used
> > for DP format output.
> >
> > @@ -75,9 +69,75 @@ required:
> > - interrupts
> > - clocks
> > - clock-names
> > - - ports
> >
> > -additionalProperties: false
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: thead,th1520-dc8200
> > + then:
> > + properties:
> > + clocks:
> > + items:
> > + - description: DC Core clock
> > + - description: DMA AXI bus clock
> > + - description: Configuration AHB bus clock
> > + - description: Pixel clock of output 0
> > + - description: Pixel clock of output 1
> > +
> > + clock-names:
> > + items:
> > + - const: core
> > + - const: axi
> > + - const: ahb
> > + - const: pix0
> > + - const: pix1
> > +
> > + resets:
> > + items:
> > + - description: DC Core reset
> > + - description: DMA AXI bus reset
> > + - description: Configuration AHB bus reset
> > +
> > + reset-names:
> > + items:
> > + - const: core
> > + - const: axi
> > + - const: ahb
> > +
> > + required:
> > + - ports
> > +
> > + else:
> > + properties:
> > + clocks:
> > + items:
> > + - description: Bus clock that gates register access
> > + - description: Pixel clock divider for display timing
>
> Please don't make compatible-specific description strings for
> individual compatibles, and keep these descriptions outside of the if.
> The compatible-specific part should be used to specify what's required
> for the specific SoC, for dt validation purpose.
>
> BTW if the clock is both the working clock and bus clock for the
> controller, I suggest listing it twice, except if the IP core is
> provided without a dedicated core clock (in the case I suggest to use
> "bus" only).
I agree. If the same clock is provided to two+ ports on the IP, that
should still be two+ clocks in the devicetree.
>
> Here's an example for "listing it twice":
> ```
> clocks = <&clk DCU_GATE>, <&clk DCU_GATE>, <&clk DCUP_DIV>;
> clock-names = "core", "bus", "pix0";
> ```
>
> Well nonetheless the name "core" does not match the description "Bus
> clock that gates register access".
>
> Thanks,
> Icenowy
>
> > +
> > + clock-names:
> > + items:
> > + - const: core
> > + - const: pix0
> > +
> > + resets:
> > + maxItems: 1
> > + description:
> > + Reset line for the display controller.
> > +
> > + reset-names:
> > + items:
> > + - const: core
> > +
> > + required:
> > + - port
> > +
> > + not:
> > + required:
> > + - ports
> > +
> > +unevaluatedProperties: false
> >
> > examples:
> > - |
> > @@ -120,3 +180,24 @@ examples:
> > };
> > };
> > };
> > +
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> > + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
> > +
> > + display@40260000 {
> > + compatible = "verisilicon,dc";
> > + reg = <0x40260000 0x20000>;
> > + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk DCU_GATE>, <&clk DCUP_DIV>;
> > + clock-names = "core", "pix0";
> > + resets = <&sys MA35D1_RESET_DISP>;
> > + reset-names = "core";
> > +
> > + port {
> > + dpi_out: endpoint {
> > + remote-endpoint = <&panel_in>;
> > + };
> > + };
> > + };
signature.asc
Description: PGP signature
