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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing explicit array length constraints in variant-specific conditional blocks.
- [High] Generic catch-all compatible string enforces SoC-specific limitations and leaves array contents unvalidated.
- [Medium] Incomplete schema topology enforcement allows conflicting 'port' and 'ports' properties.
--

commit e03be5624915d85b3073bbb37ac0fe652c0887cc
Author: Joey Lu <a0987203069@gmail.com>

dt-bindings: display: verisilicon,dc: generalize for single-output variants

This commit updates the verisilicon,dc display controller binding to support
single-output variants like the Verisilicon DCU Lite. It transitions the
schema to use an allOf/if-then-else structure to differentiate between the
existing dual-port th1520-dc8200 and a new generic verisilicon,dc compatible.
The top-level constraints are relaxed, with variant-specific rules intended
to be enforced in the conditional blocks.

> diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> index 9dc35ab973f20..3a814c2e083ee 100644
> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml

[ ... ]

> @@ -75,9 +69,75 @@ required:
> -  - 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

Does JSON schema validation require explicit minItems and maxItems here to
override the top-level ranges?

At the top level, clocks has minItems: 2 and maxItems: 5, and resets has
minItems: 1 and maxItems: 3. Providing an items array inside a conditional
block does not override those top-level ranges unless explicitly redefined.

Could a device tree node for the thead variant incorrectly pass validation
with only 2 clocks or 1 reset, completely bypassing the intended strict
hardware constraints?

> +
> +      required:
> +        - ports

Since the top-level schema now evaluates the port property globally, should
this branch explicitly forbid it?

Without rejecting port, for example by using properties: { port: false } or
not: { required: [port] }, could a malformed device tree node for the thead
variant define both port and ports simultaneously without triggering a schema
validation error?

> +
> +    else:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Bus clock that gates register access
> +            - description: Pixel clock divider for display timing
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: pix0

The top-level rules allow up to maxItems: 5 for clock-names, but this block
only lists 2 items without setting an explicit maxItems: 2 constraint.

Could a node supply 3 to 5 clocks with unconstrained, undocumented string
names that completely bypass standard schema validation?

> +
> +        resets:
> +          maxItems: 1
> +          description:
> +            Reset line for the display controller.

This branch represents a generic fallback for verisilicon,dc, but caps resets
at 1, which appears to be a Nuvoton-specific constraint.

Will this cause validation failures for future hardware integrations of this
IP that require multiple resets?

Should this binding use a specific compatible string for the Nuvoton hardware,
such as nuvoton,ma35d1-dcu, instead of enforcing these limits on a generic
catch-all string?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519055114.1886525-1-a0987203069@gmail.com?part=1