Re: [PATCH v3 1/4] dt-bindings: display: renesas,rzg2l-du: Document RZ/G2UL DU bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

On Tue, Aug 13, 2024 at 10:32:20AM -0600, Rob Herring wrote:
> On Mon, Aug 05, 2024 at 04:52:35PM +0100, Biju Das wrote:
> > Document DU found in RZ/G2UL SoC. The DU block is identical to RZ/G2L
> > SoC, but has only DPI interface.
> > 
> > While at it, add missing required property port@1 for RZ/G2L and RZ/V2L
> > SoCs. Currently there is no user for the DPI interface and hence there
> > won't be any ABI breakage for adding port@1 as required property for
> > RZ/G2L and RZ/V2L SoCs.
> > 
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > v2->v3:
> >  * Replaced ports->port property for RZ/G2UL as it supports only DPI.
> >    and retained ports property for RZ/{G2L,V2L} as it supports both DSI
> >    and DPI output interface.
> 
> Why? Having port and ports is just a needless complication.

I agree that making the ports node mandatory, even when the device has a
single port, will simplify the bindings. In hindsight we should never
have made ports optional, but that can't be changed.

> >  * Added missing blank line before example.
> >  * Dropped tags from Conor and Geert as there are new changes.
> > v1->v2:
> >  * Updated commit description related to non ABI breakage.
> >  * Added Ack from Conor.
> > ---
> >  .../bindings/display/renesas,rzg2l-du.yaml    | 35 +++++++++++++++++--
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > index 08e5b9478051..ca01bf26c4c0 100644
> > --- a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > @@ -18,6 +18,7 @@ properties:
> >    compatible:
> >      oneOf:
> >        - enum:
> > +          - renesas,r9a07g043u-du # RZ/G2UL
> >            - renesas,r9a07g044-du # RZ/G2{L,LC}
> >        - items:
> >            - enum:
> > @@ -60,8 +61,9 @@ properties:
> >          $ref: /schemas/graph.yaml#/properties/port
> >          unevaluatedProperties: false
> >  
> > -    required:
> > -      - port@0
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    description: Connection to the DU output video port.
> >  
> >      unevaluatedProperties: false
> >  
> > @@ -83,11 +85,38 @@ required:
> >    - clock-names
> >    - resets
> >    - power-domains
> > -  - ports
> >    - renesas,vsps
> >  
> >  additionalProperties: false
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,r9a07g043u-du
> > +    then:
> > +      properties:
> > +        port:
> > +          description: DPI
> 
> This is equivalent to 'port@0'. IMO, this case should have a 'port@1' 
> node so that DPI interface is *always* the same port.

That's what Biju did in the previous version, and I recommended to
number the ports based on hardware indices, not types. Mapping port
numbers to the hardware documentation makes it more consistent for DT
writers, makes the logic simpler to understand (in my opinion, based on
my experience with the R-Car DU) on the driver side, but most
importantly, type-based numbering wouldn't scale as SoCs could have
multiple ports of the same type (we've seen that happening with R-Car).

> > +
> > +      required:
> > +        - port
> > +    else:
> > +      properties:
> > +        ports:
> > +          properties:
> > +            port@0:
> > +              description: DSI
> > +            port@1:
> > +              description: DPI
> > +
> > +          required:
> > +            - port@0
> > +            - port@1
> > +      required:
> > +        - ports
> > +
> >  examples:
> >    # RZ/G2L DU
> >    - |

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux