Re: [PATCH v3 2/9] dt-bindings: display: add verisilicon,dc

Icenowy Zheng
2025-11-26T09:51:19Z

在 2025-11-24星期一的 12:01 +0100，Krzysztof Kozlowski写道：
> On 24/11/2025 11:52, Icenowy Zheng wrote:
> > Verisilicon has a series of display controllers prefixed with DC
> > and
> > with self-identification facility like their GC series GPUs.
> >
> > Add a device tree binding for it.
> >
> > Depends on the specific DC model, it can have either one or two
> > display
> > outputs, and each display output could be set to DPI signal or "DP"
> > signal (which seems to be some plain parallel bus to HDMI
> > controllers).
> > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> > Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
>
> Wrong DCO chain order. You send it as icenowy.me, so this must be
> last
> SoB. This identity is the last one certifying DCO. Please kindly read
> submitting patches, so you know what you are certifying here.
>
> > ---
> > Changes in v3:
> > - Added SoC-specific compatible string, and arm the binding with
> > clock /
> >   port checking for the specific SoC (with a 2-output DC).
> >
> > Changes in v2:
> > - Fixed misspelt "versilicon" in title.
> > - Moved minItems in clock properties to be earlier than items.
> > - Re-aligned multi-line clocks and resets in example.
> >
> >  .../bindings/display/verisilicon,dc.yaml      | 146
> > ++++++++++++++++++
> >  1 file changed, 146 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > new file mode 100644
> > index 0000000000000..522a544498bea
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > @@ -0,0 +1,146 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/verisilicon,dc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Verisilicon DC-series display controllers
> > +
> > +maintainers:
> > +  - Icenowy Zheng <uwu@icenowy.me>
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^display@[0-9a-f]+$"
> > +
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - thead,th1520-dc8200
> > +      - const: verisilicon,dc
>
> I do not see any explanation of exception for generic compatibles,
> maybe
> except "self-identification" remark. Rob already pointed this out, so
> be
> explicit in commit msg why you are using a generic compatible.

Well I only get the meaning of "a SoC specific compatible is required"
in his review message.

I think my binding now requires both a SoC-specific compatible and a
generic compatible, which should be okay to satisfy Rob's original
review.

>
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 4
>
> This is not flexible. Device either has or has not these clocks.

The existence of all these clocks are verified by diagrams in manuals
of two different SoCs with DC8200 (T-Head TH1520 and StarFive JH7110).

Maybe a explicit `maxItems: 5` is needed here, but as my DT passes
dtbs_check, I don't think it's necessary?

Or maybe I should drop the flexibility now and use a `minItems: 5` here
(and leave DC8000 support as another story)? (The Eswin EIC7700 manual
does not have a diagram showing external connections of the DC, like
the two SoCs I mentioned above).

>
> > +    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
> > +
>
>
>
> Best regards,
> Krzysztof