Re: [PATCH v2] dt-bindings: display: renesas, du: Provide bindings for r8a779a0

Hi Rob,

On Tue, Sep 7, 2021 at 8:45 PM Rob Herring robh@kernel.org wrote:

On Mon, Sep 06, 2021 at 10:13:07AM +0200, Geert Uytterhoeven wrote:

...

On Thu, Sep 2, 2021 at 1:39 AM Kieran Bingham kieran.bingham@ideasonboard.com wrote:

...

From: Kieran Bingham kieran.bingham+renesas@ideasonboard.com

Extend the Renesas DU display bindings to support the r8a779a0 V3U.

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com

---

v2:

• Collected Laurent's tag
• Remove clock-names requirement
• Specify only a single clock

Thanks for the update!

...

--- a/Documentation/devicetree/bindings/display/renesas,du.yaml +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml @@ -39,6 +39,7 @@ properties: - renesas,du-r8a77980 # for R-Car V3H compatible DU - renesas,du-r8a77990 # for R-Car E3 compatible DU - renesas,du-r8a77995 # for R-Car D3 compatible DU

•  - renesas,du-r8a779a0 # for R-Car V3U compatible DU
  
  

  reg: maxItems: 1

@@ -773,6 +774,55 @@ allOf: - reset-names - renesas,vsps

• • if:

•  properties:
  

•    compatible:
  

•      contains:
  

•        enum:
  

•          - renesas,du-r8a779a0
  

• then:

•  properties:
  

•    clocks:
  

•      items:
  

•        - description: Functional clock
  

•    clock-names:
  

•      maxItems: 1
  

•      items:
  

•        - const: du
  

•    interrupts:
  

•      maxItems: 2
  

•    resets:
  

•      maxItems: 1
  

•    reset-names:
  

•      items:
  

•        - const: du.0
  

This is now inconsistent with clock-names, which doesn't use a suffix.

But it is consistent with all the other cases of 'reset-names'. The problem is 'clock-names' is not consistent and should be 'du.0'.

True.

Ideally, the if/them schemas should not be defining the names. That should be at the top level and the if/them schema just limits the number of entries. That's not always possible, but I think is for clocks and resets in this case.

It's a bit tricky. For clocks, there's usually one clock per channel, but not always. Plus clocks for external inputs, if present. For resets, there's one reset for a group of channels, with the number of channels in a group depending on the SoC family. And then there's the special casing for SoCs where there's a gap in the channel numbering...

Still wondering if it would be better to have one device node per channel, and companion links...

Gr{oetje,eeting}s,

Geert