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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 26/11/2025 10:50, Icenowy Zheng wrote:
>>> +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.

You will get then the same questions for me - what justifies generic
compatible. You should be on this explicit, because otherwise people
misinterpret some commits and patches, and they think the generic
compatible is allowed for them as well.

> 
>>
>>> +
>>> +  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

So not flexible, then:

> 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?

No, drop minItems only.

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

You document here only the devices explicitly mentioned in the binding.
You cannot add here constraints or clocks for some device which is not
in the binding and I see only th1520 in the binding.

Best regards,
Krzysztof