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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191216183410.akrmytbvaeg37wms@gilmour.lan>
Date:   Mon, 16 Dec 2019 19:34:10 +0100
From:   Maxime Ripard <mripard@...nel.org>
To:     Jagan Teki <jagan@...rulasolutions.com>
Cc:     Chen-Yu Tsai <wens@...e.org>, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Michael Trimarchi <michael@...rulasolutions.com>,
        Icenowy Zheng <icenowy@...c.io>,
        linux-sunxi <linux-sunxi@...glegroups.com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-amarula <linux-amarula@...rulasolutions.com>
Subject: Re: [PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI
 controller

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@...nel.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@...nel.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@...nel.org>
> > > > > Signed-off-by: Jagan Teki <jagan@...rulasolutions.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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)