Re: [PATCH v3 1/6] dt-bindings: lcdif: Add i.MX93 LCDIF support

[Alexander Stein]

Hi Liu,

Am Mittwoch, 15. Februar 2023, 08:49:56 CET schrieb Liu Ying:
> On Wed, 2023-02-15 at 08:26 +0100, Alexander Stein wrote:
> > Hi Liu,
> 
> Hi Alexander,
> 
> > thanks for the update.
> 
> Thanks for the review.
> 
> > Am Montag, 13. Februar 2023, 09:56:07 CET schrieb Liu Ying:
> > > There is one LCDIF embedded in i.MX93 SoC to connect with
> > > MIPI DSI controller through LCDIF cross line pattern(controlled
> > > by mediamix blk-ctrl) or connect with LVDS display bridge(LDB)
> > > directly or connect with a parallel display through parallel
> > > display format(also controlled by mediamix blk-ctrl).  i.MX93
> > > LCDIF IP is essentially the same to i.MX8MP LCDIF IP.  Add device
> > > tree binding for i.MX93 LCDIF.
> > > 
> > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > > Reviewed-by: Marek Vasut <marex@xxxxxxx>
> > > Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> > > ---
> > > v2->v3:
> > > * No change.
> > > 
> > > v1->v2:
> > > * Add Krzysztof's A-b and Marek's R-b tags on patch 1/6.
> > > 
> > >  Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 7
> > > 
> > > ++++++-
> > > 
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> > > b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index
> > > 75b4efd70ba8..fc11ab5fc465 100644
> > > --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> > > +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> > > 
> > > @@ -21,6 +21,7 @@ properties:
> > >            - fsl,imx28-lcdif
> > >            - fsl,imx6sx-lcdif
> > >            - fsl,imx8mp-lcdif
> > > 
> > > +          - fsl,imx93-lcdif
> > > 
> > >        - items:
> > >            - enum:
> > >                - fsl,imx6sl-lcdif
> > > 
> > > @@ -88,7 +89,9 @@ allOf:
> > >        properties:
> > >          compatible:
> > >            contains:
> > > -            const: fsl,imx8mp-lcdif
> > > +            enum:
> > > +              - fsl,imx8mp-lcdif
> > > +              - fsl,imx93-lcdif
> > > 
> > >      then:
> > >        properties:
> > >          clocks:
> > > @@ -107,6 +110,7 @@ allOf:
> > >                enum:
> > >                  - fsl,imx6sx-lcdif
> > >                  - fsl,imx8mp-lcdif
> > > 
> > > +                - fsl,imx93-lcdif
> > > 
> > >      then:
> > >        properties:
> > >          clocks:
> > > @@ -123,6 +127,7 @@ allOf:
> > >                - fsl,imx8mm-lcdif
> > >                - fsl,imx8mn-lcdif
> > >                - fsl,imx8mp-lcdif
> > > 
> > > +              - fsl,imx93-lcdif
> > > 
> > >      then:
> > >        required:
> > >          - power-domains
> > 
> > I would have expected that fsl,imx93-lcdif supports up to 3 endpoints
> > (MIPI
> > DSI, LVDS, and parallel) in a 'ports' subnode. But this binding only
> > supports
> > a single 'port' sub-node. Also an example for this case might be very
> > helpful.
> 
> The port node allows multiple endpoints(See graph.yaml[1]).  It's
> enough to use the existing port node instead of using ports node.

Ah, I wasn't aware of that possibility for OF graph. Yep, agreed then it's 
enough.

> For i.MX93 LCDIF, the port node will be something like this:
> --------------------8<--------------------------
> port {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         lcdif_to_pdfc: endpoint@0 {
>                 reg = <0>;
>         };
> 
>         lcdif_to_ldb: endpoint@1 {
>                 reg = <1>;
>         };
> 
>         lcdif_to_cross_line_pattern: endpoint@2 {
>                 reg = <2>;
>         };
> };
> --------------------8<--------------------------
> 
> Looks like it's not necessary to add a specifc example for i.MX93
> LCDIF.

Fine by me.
Reviewed-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>

> [1]
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/graph
> .yaml#L48
> 
> Regards,
> Liu Ying

TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider