Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <87d0ahzr9d.wl-kuninori.morimoto.gx@renesas.com>
 <20200219161732.GB25095@bogus> <874kvmt355.wl-kuninori.morimoto.gx@renesas.com>
In-Reply-To: <874kvmt355.wl-kuninori.morimoto.gx@renesas.com>
From:   Rob Herring <robh@kernel.org>
Date:   Thu, 20 Feb 2020 13:33:56 -0600
Message-ID: <CAL_Jsq+VEQj9Nkyo_85RM3Ku1-D73_ot5BTAjidnJzJv7r1_Sw@mail.gmail.com>
Subject: Re: [PATCH v2] ASoC: dt-bindings: renesas,rsnd: switch to yaml base Documentation
To:     Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Linux-ALSA <alsa-devel@alsa-project.org>,
        devicetree@vger.kernel.org,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 19, 2020 at 8:16 PM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> Hi Rob
>
> Thank you for your review
>
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > >
> > > This patch switches from .txt base to .yaml base Document.
> > > It is still keeping detail explanations at .txt
> > >
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > ---
> (snip)
> > > +  compatible:
> > > +    oneOf:
> > > +      # for Gen1 SoC
> > > +      - items:
> > > +        - enum:
> >
> > nit: Should be indented 2 more spaces.
>
> I couldn't understand this.
> Do you mean like this ??
>
>    compatible:
>      oneOf:
>        # for Gen1 SoC
>        - items:
> =>         - enum:

Yes.

>
> > > +  clock-frequency:
> > > +    description: for audio_clkout0/1/2/3
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> > This already has a common definition and this conflicts with that.
> > 'clock-frequency' is a single uint32 or uint64.
>
> This needs clock array. Like this
>
>         clock-frequency = <12288000 11289600>;

Sorry, but the type is already defined in the spec. You'll still get
warnings from the common schema and you can't override that here.

Not sure what to suggest. Leave it with a fixme or move to
assigned-clocks-rates instead?

> > > +  # For OF-graph
> > > +  port:
> > > +    description: OF-Graph subnode
> > > +    type: object
> > > +    properties:
> > > +      reg:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> >
> > No unit-address for 'port', so you don't need 'reg' here.
>
> But I got warning without reg ?

Yeah, because of your $ref from ports.

> And, renesas,rsnd and/or simple-card sometimes needs reg for port
> to handle sound path.
>
> > > +  # For multi OF-graph
> > > +  ports:
> > > +    description: multi OF-Graph subnode
> > > +    type: object
> > > +    patternProperties:
> > > +      "port(@.*)?":
> >
> > ^port(@[0-9a-f])?$"
> >
> > Perhaps there's max number of ports that's less than 0xf?
> >
> > > +        $ref: "#properties/port"
> >
> > Would be more simple to just always have 'ports'.
>
> Having "ports" or "port" are case-by-case, not always.

Why?

This:

port {};

and this:

ports {
  port {};
};

Are treated the same. It's perfectly valid to have 'ports' with a single port.

Rob