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

From: Jagan Teki
Date: Mon Dec 16 2019 - 11:59:23 EST


On Mon, Dec 16, 2019 at 4:50 PM Maxime Ripard <mripard@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> > > > ---
> > > > 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