[v12,1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

Message ID 20191203134816.5319-2-jagan@amarulasolutions.com
State New
Headers show
Series
  • drm/sun4i: Allwinner A64 MIPI-DSI support
Related show

Commit Message

Jagan Teki Dec. 3, 2019, 1:48 p.m. UTC
The MIPI DSI controller in Allwinner A64 is similar to A33.

But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
to have separate compatible for A64 on the same driver.

DSI_SCLK uses mod clock-names on dt-bindings, so the same
is not required for A64.

On that note
- A64 require minimum of 1 clock like the bus clock
- A33 require minimum of 2 clocks like both bus, mod clocks

So, update dt-bindings so-that it can document both A33,
A64 bindings requirements.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v12:
- Use 'enum' instead of oneOf+const

 .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

Maxime Ripard Dec. 4, 2019, 1:36 p.m. UTC | #1
On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> The MIPI DSI controller in Allwinner A64 is similar to A33.
>
> But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> to have separate compatible for A64 on the same driver.
>
> DSI_SCLK uses mod clock-names on dt-bindings, so the same
> is not required for A64.
>
> On that note
> - A64 require minimum of 1 clock like the bus clock
> - A33 require minimum of 2 clocks like both bus, mod clocks
>
> So, update dt-bindings so-that it can document both A33,
> A64 bindings requirements.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v12:
> - Use 'enum' instead of oneOf+const
>
>  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> index dafc0980c4fa..b91446475f35 100644
> --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> @@ -15,7 +15,9 @@ properties:
>    "#size-cells": true
>
>    compatible:
> -    const: allwinner,sun6i-a31-mipi-dsi
> +    enum:
> +      - allwinner,sun6i-a31-mipi-dsi
> +      - allwinner,sun50i-a64-mipi-dsi
>
>    reg:
>      maxItems: 1
> @@ -24,6 +26,8 @@ properties:
>      maxItems: 1
>
>    clocks:
> +    minItems: 1
> +    maxItems: 2
>      items:
>        - description: Bus Clock
>        - description: Module Clock
> @@ -63,13 +67,25 @@ required:
>    - reg
>    - interrupts
>    - clocks
> -  - clock-names
>    - phys
>    - phy-names
>    - resets
>    - vcc-dsi-supply
>    - port
>
> +allOf:
> +  - if:
> +      properties:
> +         compatible:
> +           contains:
> +             const: allwinner,sun6i-a31-mipi-dsi
> +      then:
> +        properties:
> +          clocks:
> +            minItems: 2
> +        required:
> +          - clock-names
> +

Your else condition should check that the number of clocks items is 1
on the A64

Maxime
Jagan Teki Dec. 16, 2019, 11:07 a.m. UTC | #2
On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > The MIPI DSI controller in Allwinner A64 is similar to A33.
> >
> > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > to have separate compatible for A64 on the same driver.
> >
> > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > is not required for A64.
> >
> > On that note
> > - A64 require minimum of 1 clock like the bus clock
> > - A33 require minimum of 2 clocks like both bus, mod clocks
> >
> > So, update dt-bindings so-that it can document both A33,
> > A64 bindings requirements.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v12:
> > - Use 'enum' instead of oneOf+const
> >
> >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > index dafc0980c4fa..b91446475f35 100644
> > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > @@ -15,7 +15,9 @@ properties:
> >    "#size-cells": true
> >
> >    compatible:
> > -    const: allwinner,sun6i-a31-mipi-dsi
> > +    enum:
> > +      - allwinner,sun6i-a31-mipi-dsi
> > +      - allwinner,sun50i-a64-mipi-dsi
> >
> >    reg:
> >      maxItems: 1
> > @@ -24,6 +26,8 @@ properties:
> >      maxItems: 1
> >
> >    clocks:
> > +    minItems: 1
> > +    maxItems: 2
> >      items:
> >        - description: Bus Clock
> >        - description: Module Clock
> > @@ -63,13 +67,25 @@ required:
> >    - reg
> >    - interrupts
> >    - clocks
> > -  - clock-names
> >    - phys
> >    - phy-names
> >    - resets
> >    - vcc-dsi-supply
> >    - port
> >
> > +allOf:
> > +  - if:
> > +      properties:
> > +         compatible:
> > +           contains:
> > +             const: allwinner,sun6i-a31-mipi-dsi
> > +      then:
> > +        properties:
> > +          clocks:
> > +            minItems: 2
> > +        required:
> > +          - clock-names
> > +
>
> Your else condition should check that the number of clocks items is 1
> on the A64

But the minItems mentioned as 1 in clocks, which is unchanged number
by default. doesn't it sufficient?
Maxime Ripard Dec. 16, 2019, 11:20 a.m. UTC | #3
On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > >
> > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > to have separate compatible for A64 on the same driver.
> > >
> > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > is not required for A64.
> > >
> > > On that note
> > > - A64 require minimum of 1 clock like the bus clock
> > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > >
> > > So, update dt-bindings so-that it can document both A33,
> > > A64 bindings requirements.
> > >
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v12:
> > > - Use 'enum' instead of oneOf+const
> > >
> > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > index dafc0980c4fa..b91446475f35 100644
> > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > @@ -15,7 +15,9 @@ properties:
> > >    "#size-cells": true
> > >
> > >    compatible:
> > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > +    enum:
> > > +      - allwinner,sun6i-a31-mipi-dsi
> > > +      - allwinner,sun50i-a64-mipi-dsi
> > >
> > >    reg:
> > >      maxItems: 1
> > > @@ -24,6 +26,8 @@ properties:
> > >      maxItems: 1
> > >
> > >    clocks:
> > > +    minItems: 1
> > > +    maxItems: 2
> > >      items:
> > >        - description: Bus Clock
> > >        - description: Module Clock
> > > @@ -63,13 +67,25 @@ required:
> > >    - reg
> > >    - interrupts
> > >    - clocks
> > > -  - clock-names
> > >    - phys
> > >    - phy-names
> > >    - resets
> > >    - vcc-dsi-supply
> > >    - port
> > >
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +         compatible:
> > > +           contains:
> > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > +      then:
> > > +        properties:
> > > +          clocks:
> > > +            minItems: 2
> > > +        required:
> > > +          - clock-names
> > > +
> >
> > Your else condition should check that the number of clocks items is 1
> > on the A64
>
> But the minItems mentioned as 1 in clocks, which is unchanged number
> by default. doesn't it sufficient?

In the main schema, it's said that the clocks property can have one or
two elements (to cover the A31 case that has one, and the A64 case
that has 2).

This is fine.

Later on, you enforce that the A64 has two elements, and this is fine
too.

However, you never check that on the A31 you only have one clock, and
you could very well have two and no one would notice.

Maxime
Jagan Teki Dec. 16, 2019, 4:59 p.m. UTC | #4
On Mon, Dec 16, 2019 at 4:50 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> > On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > > >
> > > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > > to have separate compatible for A64 on the same driver.
> > > >
> > > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > > is not required for A64.
> > > >
> > > > On that note
> > > > - A64 require minimum of 1 clock like the bus clock
> > > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > > >
> > > > So, update dt-bindings so-that it can document both A33,
> > > > A64 bindings requirements.
> > > >
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > ---
> > > > Changes for v12:
> > > > - Use 'enum' instead of oneOf+const
> > > >
> > > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > index dafc0980c4fa..b91446475f35 100644
> > > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > @@ -15,7 +15,9 @@ properties:
> > > >    "#size-cells": true
> > > >
> > > >    compatible:
> > > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > > +    enum:
> > > > +      - allwinner,sun6i-a31-mipi-dsi
> > > > +      - allwinner,sun50i-a64-mipi-dsi
> > > >
> > > >    reg:
> > > >      maxItems: 1
> > > > @@ -24,6 +26,8 @@ properties:
> > > >      maxItems: 1
> > > >
> > > >    clocks:
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > >      items:
> > > >        - description: Bus Clock
> > > >        - description: Module Clock
> > > > @@ -63,13 +67,25 @@ required:
> > > >    - reg
> > > >    - interrupts
> > > >    - clocks
> > > > -  - clock-names
> > > >    - phys
> > > >    - phy-names
> > > >    - resets
> > > >    - vcc-dsi-supply
> > > >    - port
> > > >
> > > > +allOf:
> > > > +  - if:
> > > > +      properties:
> > > > +         compatible:
> > > > +           contains:
> > > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > > +      then:
> > > > +        properties:
> > > > +          clocks:
> > > > +            minItems: 2
> > > > +        required:
> > > > +          - clock-names
> > > > +
> > >
> > > Your else condition should check that the number of clocks items is 1
> > > on the A64
> >
> > But the minItems mentioned as 1 in clocks, which is unchanged number
> > by default. doesn't it sufficient?
>
> In the main schema, it's said that the clocks property can have one or
> two elements (to cover the A31 case that has one, and the A64 case
> that has 2).
>
> This is fine.
>
> Later on, you enforce that the A64 has two elements, and this is fine
> too.

Actually A31 case has 2 and A64 case has 1.

>
> However, you never check that on the A31 you only have one clock, and
> you could very well have two and no one would notice.

I did check A31 case for 2 but not in A64. this is what you mean? so
adding A64 check like below would fine?

allOf:
  - if:
      properties:
         compatible:
           contains:
             const: allwinner,sun6i-a31-mipi-dsi
      then:
        properties:
          clocks:
            minItems: 2
        required:
          - clock-names
  - if:
      properties:
         compatible:
           contains:
             const: allwinner,sun50i-a64-mipi-dsi
      then:
        properties:
          clocks:
            minItems: 1
Maxime Ripard Dec. 16, 2019, 6:34 p.m. UTC | #5
On Mon, Dec 16, 2019 at 10:29:08PM +0530, Jagan Teki wrote:
> On Mon, Dec 16, 2019 at 4:50 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> > > On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > >
> > > > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > > > >
> > > > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > > > to have separate compatible for A64 on the same driver.
> > > > >
> > > > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > > > is not required for A64.
> > > > >
> > > > > On that note
> > > > > - A64 require minimum of 1 clock like the bus clock
> > > > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > > > >
> > > > > So, update dt-bindings so-that it can document both A33,
> > > > > A64 bindings requirements.
> > > > >
> > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > > Changes for v12:
> > > > > - Use 'enum' instead of oneOf+const
> > > > >
> > > > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > index dafc0980c4fa..b91446475f35 100644
> > > > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > @@ -15,7 +15,9 @@ properties:
> > > > >    "#size-cells": true
> > > > >
> > > > >    compatible:
> > > > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > > > +    enum:
> > > > > +      - allwinner,sun6i-a31-mipi-dsi
> > > > > +      - allwinner,sun50i-a64-mipi-dsi
> > > > >
> > > > >    reg:
> > > > >      maxItems: 1
> > > > > @@ -24,6 +26,8 @@ properties:
> > > > >      maxItems: 1
> > > > >
> > > > >    clocks:
> > > > > +    minItems: 1
> > > > > +    maxItems: 2
> > > > >      items:
> > > > >        - description: Bus Clock
> > > > >        - description: Module Clock
> > > > > @@ -63,13 +67,25 @@ required:
> > > > >    - reg
> > > > >    - interrupts
> > > > >    - clocks
> > > > > -  - clock-names
> > > > >    - phys
> > > > >    - phy-names
> > > > >    - resets
> > > > >    - vcc-dsi-supply
> > > > >    - port
> > > > >
> > > > > +allOf:
> > > > > +  - if:
> > > > > +      properties:
> > > > > +         compatible:
> > > > > +           contains:
> > > > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > > > +      then:
> > > > > +        properties:
> > > > > +          clocks:
> > > > > +            minItems: 2
> > > > > +        required:
> > > > > +          - clock-names
> > > > > +
> > > >
> > > > Your else condition should check that the number of clocks items is 1
> > > > on the A64
> > >
> > > But the minItems mentioned as 1 in clocks, which is unchanged number
> > > by default. doesn't it sufficient?
> >
> > In the main schema, it's said that the clocks property can have one or
> > two elements (to cover the A31 case that has one, and the A64 case
> > that has 2).
> >
> > This is fine.
> >
> > Later on, you enforce that the A64 has two elements, and this is fine
> > too.
>
> Actually A31 case has 2 and A64 case has 1.
>
> >
> > However, you never check that on the A31 you only have one clock, and
> > you could very well have two and no one would notice.
>
> I did check A31 case for 2 but not in A64. this is what you mean? so
> adding A64 check like below would fine?
>
> allOf:
>   - if:
>       properties:
>          compatible:
>            contains:
>              const: allwinner,sun6i-a31-mipi-dsi

You need a new line here

>       then:

This is the wrong level of indentation, it needs to be at the same level than if

>         properties:
>           clocks:
>             minItems: 2

Newline

>         required:
>           - clock-names

Newline

>   - if:
>       properties:
>          compatible:
>            contains:
>              const: allwinner,sun50i-a64-mipi-dsi
>       then:
>         properties:
>           clocks:
>             minItems: 1

(and same thing here).

But yeah, otherwise that's what I meant

Maxime
Jagan Teki Dec. 17, 2019, 12:06 p.m. UTC | #6
On Tue, Dec 17, 2019 at 12:04 AM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Dec 16, 2019 at 10:29:08PM +0530, Jagan Teki wrote:
> > On Mon, Dec 16, 2019 at 4:50 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> > > > On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > >
> > > > > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > > > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > > > > >
> > > > > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > > > > to have separate compatible for A64 on the same driver.
> > > > > >
> > > > > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > > > > is not required for A64.
> > > > > >
> > > > > > On that note
> > > > > > - A64 require minimum of 1 clock like the bus clock
> > > > > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > > > > >
> > > > > > So, update dt-bindings so-that it can document both A33,
> > > > > > A64 bindings requirements.
> > > > > >
> > > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > ---
> > > > > > Changes for v12:
> > > > > > - Use 'enum' instead of oneOf+const
> > > > > >
> > > > > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > > > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > > index dafc0980c4fa..b91446475f35 100644
> > > > > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > > @@ -15,7 +15,9 @@ properties:
> > > > > >    "#size-cells": true
> > > > > >
> > > > > >    compatible:
> > > > > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > > > > +    enum:
> > > > > > +      - allwinner,sun6i-a31-mipi-dsi
> > > > > > +      - allwinner,sun50i-a64-mipi-dsi
> > > > > >
> > > > > >    reg:
> > > > > >      maxItems: 1
> > > > > > @@ -24,6 +26,8 @@ properties:
> > > > > >      maxItems: 1
> > > > > >
> > > > > >    clocks:
> > > > > > +    minItems: 1
> > > > > > +    maxItems: 2
> > > > > >      items:
> > > > > >        - description: Bus Clock
> > > > > >        - description: Module Clock
> > > > > > @@ -63,13 +67,25 @@ required:
> > > > > >    - reg
> > > > > >    - interrupts
> > > > > >    - clocks
> > > > > > -  - clock-names
> > > > > >    - phys
> > > > > >    - phy-names
> > > > > >    - resets
> > > > > >    - vcc-dsi-supply
> > > > > >    - port
> > > > > >
> > > > > > +allOf:
> > > > > > +  - if:
> > > > > > +      properties:
> > > > > > +         compatible:
> > > > > > +           contains:
> > > > > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > > > > +      then:
> > > > > > +        properties:
> > > > > > +          clocks:
> > > > > > +            minItems: 2
> > > > > > +        required:
> > > > > > +          - clock-names
> > > > > > +
> > > > >
> > > > > Your else condition should check that the number of clocks items is 1
> > > > > on the A64
> > > >
> > > > But the minItems mentioned as 1 in clocks, which is unchanged number
> > > > by default. doesn't it sufficient?
> > >
> > > In the main schema, it's said that the clocks property can have one or
> > > two elements (to cover the A31 case that has one, and the A64 case
> > > that has 2).
> > >
> > > This is fine.
> > >
> > > Later on, you enforce that the A64 has two elements, and this is fine
> > > too.
> >
> > Actually A31 case has 2 and A64 case has 1.
> >
> > >
> > > However, you never check that on the A31 you only have one clock, and
> > > you could very well have two and no one would notice.
> >
> > I did check A31 case for 2 but not in A64. this is what you mean? so
> > adding A64 check like below would fine?
> >
> > allOf:
> >   - if:
> >       properties:
> >          compatible:
> >            contains:
> >              const: allwinner,sun6i-a31-mipi-dsi
>
> You need a new line here

I couldn't see new line before then: in example schema -
https://elixir.bootlin.com/linux/v5.5-rc2/source/Documentation/devicetree/bindings/example-schema.yaml#L208
May be a new changes for this ML or so?

>
> >       then:
>
> This is the wrong level of indentation, it needs to be at the same level than if

True, I have seen it in example schema.

>
> >         properties:
> >           clocks:
> >             minItems: 2
>
> Newline
>
> >         required:
> >           - clock-names
>
> Newline
>
> >   - if:
> >       properties:
> >          compatible:
> >            contains:
> >              const: allwinner,sun50i-a64-mipi-dsi
> >       then:
> >         properties:
> >           clocks:
> >             minItems: 1
>
> (and same thing here).
>
> But yeah, otherwise that's what I meant

Thanks for the detailed review.

Patch

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
index dafc0980c4fa..b91446475f35 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
@@ -15,7 +15,9 @@  properties:
   "#size-cells": true
 
   compatible:
-    const: allwinner,sun6i-a31-mipi-dsi
+    enum:
+      - allwinner,sun6i-a31-mipi-dsi
+      - allwinner,sun50i-a64-mipi-dsi
 
   reg:
     maxItems: 1
@@ -24,6 +26,8 @@  properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
+    maxItems: 2
     items:
       - description: Bus Clock
       - description: Module Clock
@@ -63,13 +67,25 @@  required:
   - reg
   - interrupts
   - clocks
-  - clock-names
   - phys
   - phy-names
   - resets
   - vcc-dsi-supply
   - port
 
+allOf:
+  - if:
+      properties:
+         compatible:
+           contains:
+             const: allwinner,sun6i-a31-mipi-dsi
+      then:
+        properties:
+          clocks:
+            minItems: 2
+        required:
+          - clock-names
+
 additionalProperties: false
 
 examples: