Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1629453791-10580-1-git-send-email-shengjiu.wang@nxp.com> <YSAQgywy8UW540h9@robh.at.kernel.org>
In-Reply-To: <YSAQgywy8UW540h9@robh.at.kernel.org>
From:   Shengjiu Wang <shengjiu.wang@gmail.com>
Date:   Mon, 23 Aug 2021 17:45:20 +0800
Message-ID: <CAA+D8AM1Ti5+LAwx+b5ZEy7Xi8Tx4n2Dh0ZW9faFDB04pLECRw@mail.gmail.com>
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: Add fsl,imx-dsp-rproc
 binding document
To:     Rob Herring <robh@kernel.org>
Cc:     Shengjiu Wang <shengjiu.wang@nxp.com>, ohad@wizery.com,
        bjorn.andersson@linaro.org, mathieu.poirier@linaro.org,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Sascha Hauer <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        linux-remoteproc@vger.kernel.org,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Rob

On Sat, Aug 21, 2021 at 4:28 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Aug 20, 2021 at 06:03:10PM +0800, Shengjiu Wang wrote:
> > Define the compatible string and properties needed by imx_dsp_rproc
> > driver.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  .../remoteproc/fsl,imx-dsp-rproc.yaml         | 123 ++++++++++++++++++
> >  1 file changed, 123 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/remoteproc/fsl,imx-dsp-rproc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-dsp-rproc.yaml
> > new file mode 100644
> > index 000000000000..7eba49aa959e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-dsp-rproc.yaml
> > @@ -0,0 +1,123 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/remoteproc/fsl,imx-dsp-rproc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: i.MX DSP Remoteproc Devices
> > +
> > +maintainers:
> > +  - Shengjiu Wang <shengjiu.wang@nxp.com>
> > +
> > +description:
> > +  This binding provides support for DSP processors found on i.mX family of SoCs
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,imx8qxp-hifi4
> > +      - fsl,imx8qm-hifi4
> > +      - fsl,imx8mp-hifi4
> > +      - fsl,imx8ulp-hifi4
> > +
> > +  clocks:
> > +    description: |
> > +      Main functional clock for the remote processor
>
> Don't need '|' if no formatting.

Ok,  will update it.

>
> > +    minItems: 1
> > +    maxItems: 32
> > +
> > +  clock-names:
> > +    description:
> > +      List of clock names for the remote processor.
> > +      dsp_clkx for clocks of dsp itself.
> > +      per_clkx for clocks of peripherals used by dsp.
>
> Sounds like constraints that should be a schema. But this looks like
> some random collection of clocks, not clocks for the DSP h/w.

The DSP may use some peripheral devices, then we need to
enable the clock in this driver. Using which peripherals
depends on the DSP firmware, so we didn't specify the clock
name here.

I will add below items for this.
    items:
      - const: dsp_clk1
      - const: dsp_clk2
      - const: dsp_clk3
      - const: dsp_clk4
      - const: dsp_clk5
      - const: dsp_clk6
      - const: dsp_clk7
      - const: dsp_clk8
      - const: per_clk1
      - const: per_clk2
      - const: per_clk3
      - const: per_clk4
      - const: per_clk5
      - const: per_clk6
      - const: per_clk7
      - const: per_clk8
      - const: per_clk9
      - const: per_clk10
      - const: per_clk11
      - const: per_clk12
      - const: per_clk13
      - const: per_clk14
      - const: per_clk15
      - const: per_clk16
      - const: per_clk17
      - const: per_clk18

>
> > +    minItems: 1
> > +    maxItems: 32
> > +
> > +  syscon:
>
> Vendor prefix and name it based on what it points to or is used for.

Ok, will update it.

best regards
Wang shengjiu