All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joey Lu <a0987203069@gmail.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: zhengxingda@iscas.ac.cn, maarten.lankhorst@linux.intel.com,
	mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com,
	simona@ffwll.ch, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, ychuang3@nuvoton.com, schung@nuvoton.com,
	yclu4@nuvoton.com, dri-devel@lists.freedesktop.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants
Date: Mon, 8 Jun 2026 17:44:00 +0800	[thread overview]
Message-ID: <fb9c51bb-5e17-4e62-bfab-efc648cff4d6@gmail.com> (raw)
In-Reply-To: <20260608-emotional-rapid-woodlouse-61f7b9@quoll>


On 6/8/2026 4:00 PM, Krzysztof Kozlowski wrote:
> On Mon, Jun 08, 2026 at 10:32:33AM +0800, Joey Lu wrote:
>> The existing schema hard-codes the five-clock/three-reset/dual-port
>> topology of the DC8200 IP block, preventing reuse for single-output
>> variants such as the Verisilicon DCUltraLite used in the Nuvoton MA35D1
>> SoC.
>>
>> Rework the schema so that variant-specific constraints are expressed via
>> allOf/if blocks:
>>
>> - Add nuvoton,ma35d1-dcu to the SoC-specific compatible enum.  The
>>    generic verisilicon,dc fallback remains the driver-binding string.
>> - Relax the top-level clocks/resets definitions to minItems ranges so
>>    the base schema accepts both variants.
>> - Keep ports in the global required list and keep additionalProperties
>>    tightened to unevaluatedProperties.
>> - Add an allOf/if block for thead,th1520-dc8200: five-clock (core, axi,
>>    ahb, pix0, pix1), three-reset (core, axi, ahb).
>> - Add an allOf/if block for nuvoton,ma35d1-dcu: two-clock (core, pix0),
>>    one-reset (core).
>> - Fix a stray space in the port@0 description.
>> - Add a DT example for the Nuvoton MA35D1 DCU Lite using ports/port@0.
>>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
>> ---
>>   .../bindings/display/verisilicon,dc.yaml      | 103 +++++++++++++++---
>>   1 file changed, 90 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> index 9dc35ab973f2..db0260d874c5 100644
>> --- a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> +++ b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
>> @@ -17,7 +17,8 @@ properties:
>>       items:
>>         - enum:
>>             - thead,th1520-dc8200
>> -      - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
>> +          - nuvoton,ma35d1-dcu
>> +      - const: verisilicon,dc  # DC IPs have discoverable ID/revision registers
> Why do you need to change indentation? Why introducing irrelevant
> changes to the diff?
The extra space was introduced to satisfy `yamllint`'s "too few spaces 
before comment" warning, which requires two spaces before an inline `#`. 
Since this is an unrelated change that pollutes the diff, I will revert 
it to the original single-space form.
>>   
>>     reg:
>>       maxItems: 1
>> @@ -26,6 +27,7 @@ properties:
>>       maxItems: 1
>>   
>>     clocks:
>> +    minItems: 2
>>       items:
>>         - description: DC Core clock
>>         - description: DMA AXI bus clock
> That's not true anymore. In such case the list should also be defined
> per variant and here only min/maxItems.
>
Understood. I will remove the `items:` description list from the 
top-level `clocks:` and keep only `minItems`/`maxItems`. The per-variant 
items descriptions will be moved into the allOf/if blocks.
>> @@ -34,24 +36,19 @@ properties:
>>         - description: Pixel clock of output 1
>>   
>>     clock-names:
>> -    items:
>> -      - const: core
>> -      - const: axi
>> -      - const: ahb
>> -      - const: pix0
>> -      - const: pix1
>> +    minItems: 2
>> +    maxItems: 5
>>   
>>     resets:
>> +    minItems: 1
>>       items:
>>         - description: DC Core reset
>>         - description: DMA AXI bus reset
>>         - description: Configuration AHB bus reset
>>   
>>     reset-names:
>> -    items:
>> -      - const: core
>> -      - const: axi
>> -      - const: ahb
> This stays, with minItems. Variants only need min/maxItems
>
Understood. I will restore the top-level `clock-names` and `reset-names` 
items lists and add `minItems` to each. The per-variant allOf blocks 
will only carry `minItems`/`maxItems`.
>
>> +    minItems: 1
>> +    maxItems: 3
>>   
>>     ports:
>>       $ref: /schemas/graph.yaml#/properties/ports
>> @@ -59,7 +56,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.
>>   
>> @@ -77,7 +74,60 @@ required:
>>     - clock-names
>>     - ports
>>   
>> -additionalProperties: false
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: thead,th1520-dc8200
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 5
>> +          maxItems: 5
>> +
>> +        clock-names:
>> +          items:
>> +            - const: core
>> +            - const: axi
>> +            - const: ahb
>> +            - const: pix0
>> +            - const: pix1
>> +
>> +        resets:
>> +          minItems: 3
>> +          maxItems: 3
>> +
>> +        reset-names:
> minItems: 3
Understood. I will add `minItems: 3` to `reset-names` in the 
thead,th1520-dc8200 block.
>> +          items:
>> +            - const: core
>> +            - const: axi
>> +            - const: ahb
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: nuvoton,ma35d1-dcu
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 2
>> +          maxItems: 2
>> +
>> +        clock-names:
>> +          items:
>> +            - const: core
>> +            - const: pix0
>> +
>> +        resets:
>> +          maxItems: 1
>> +
>> +        reset-names:
> maxItems: 1
Understood. I will add `maxItems: 1` to `reset-names` in the nuvoton block.
>> +          items:
>> +            - const: core
>> +
>> +unevaluatedProperties: false
> Stop making random changes to the binding.
>
> Best regards,
> Krzysztof

Understood. I will revert to `additionalProperties: false` as in the 
original binding.

Many thanks!



  reply	other threads:[~2026-06-08  9:44 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-08  2:32 [PATCH v3 0/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support Joey Lu
2026-06-08  2:32 ` [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: generalize for single-output variants Joey Lu
2026-06-08  2:32   ` [PATCH v3 1/5] dt-bindings: display: verisilicon, dc: " Joey Lu
2026-06-08  2:39   ` [PATCH v3 1/5] dt-bindings: display: verisilicon,dc: " sashiko-bot
2026-06-08  6:32   ` Icenowy Zheng
2026-06-08  9:42     ` Joey Lu
2026-06-08  8:00   ` Krzysztof Kozlowski
2026-06-08  9:44     ` Joey Lu [this message]
2026-06-08  8:02   ` Krzysztof Kozlowski
2026-06-08  9:44     ` Joey Lu
2026-06-08  2:32 ` [PATCH v3 2/5] drm/verisilicon: add register-level macros for DCU Lite Joey Lu
2026-06-08  2:32 ` [PATCH v3 3/5] drm/verisilicon: introduce per-variant hardware ops table Joey Lu
2026-06-08  2:44   ` sashiko-bot
2026-06-08  6:24   ` Icenowy Zheng
2026-06-08  9:45     ` Joey Lu
2026-06-08 10:06       ` Icenowy Zheng
2026-06-08 10:35         ` Joey Lu
2026-06-08  2:32 ` [PATCH v3 4/5] drm/verisilicon: add Nuvoton MA35D1 DCU Lite display controller support Joey Lu
2026-06-08  2:47   ` sashiko-bot
2026-06-08  6:26   ` Icenowy Zheng
2026-06-08  9:46     ` Joey Lu
2026-06-08  2:32 ` [PATCH v3 5/5] drm/verisilicon: add DCUltraLite chip identity to HWDB Joey Lu
2026-06-08  2:41   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fb9c51bb-5e17-4e62-bfab-efc648cff4d6@gmail.com \
    --to=a0987203069@gmail.com \
    --cc=airlied@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=robh@kernel.org \
    --cc=schung@nuvoton.com \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    --cc=ychuang3@nuvoton.com \
    --cc=yclu4@nuvoton.com \
    --cc=zhengxingda@iscas.ac.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.