Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add fsl,imx-dsp-rproc binding document

From: Shengjiu Wang
Date: Fri Sep 03 2021 - 03:54:55 EST


Hi Rob

On Thu, Sep 2, 2021 at 11:17 PM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Wed, Aug 25, 2021 at 03:28:35PM +0800, Shengjiu Wang wrote:
> > Define the compatible string and properties needed by imx_dsp_rproc
> > driver.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx>
> > ---
> > changes in v2:
> > - add items for clock-names
> > - change syscon to fsl,dsp-ctrl
> >
> > .../remoteproc/fsl,imx-dsp-rproc.yaml | 150 ++++++++++++++++++
> > 1 file changed, 150 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..edf6e4b8d7bb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-dsp-rproc.yaml
> > @@ -0,0 +1,150 @@
> > +# 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@xxxxxxx>
> > +
> > +description:
> > + This binding provides support for DSP processors found on i.mX family of SoCs
>
> i.MX

I will update it.

>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - fsl,imx8qxp-hifi4
> > + - fsl,imx8qm-hifi4
> > + - fsl,imx8mp-hifi4
> > + - fsl,imx8ulp-hifi4
> > +
> > + clocks:
> > + description:
> > + Main functional clock for the remote processor
> > + 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.
>
> I still don't like your random collection of clocks. What if you have
> other resources besides clocks to manage? Pin mux, resets, power
> domains, etc. It would be better if you describe the peripherals in DT
> and then link to them here. But maybe the devices themselves aren't
> visible?

Ok, I will remove these per_clks in next version.
Maybe it is better to handle them in firmware side.

>
> > + minItems: 1
> > + maxItems: 26
> > + 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
>
> Surely you can name these by function? e.g. core, bus, axi, etc.

Ok, I will update them in next version.

Thanks for reviewing.

Best regards
Wang shengjiu

>
> > + - 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
> > +
> > + fsl,dsp-ctrl:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + Phandle to syscon block which provide access for processor enablement
> > +
> > + mbox-names:
> > + items:
> > + - const: tx
> > + - const: rx
> > + - const: rxdb
> > +
> > + mboxes:
> > + description:
> > + This property is required only if the rpmsg/virtio functionality is used.
> > + List of <&phandle type channel> - 1 channel for TX, 1 channel for RX, 1 channel for RXDB.
> > + (see mailbox/fsl,mu.yaml)
> > + minItems: 1
> > + maxItems: 3
> > +
> > + firmware-name:
> > + description: |
> > + Default name of the firmware to load to the remote processor.
> > +
> > + memory-region:
> > + description:
> > + If present, a phandle for a reserved memory area that used for vdev buffer,
> > + resource table, vring region and others used by remote processor.
> > + minItems: 1
> > + maxItems: 32
> > +
> > + reg:
> > + description: |
> > + Address space for any remoteproc memories present on the SoC.
> > +
> > + power-domains:
> > + minItems: 1
> > + maxItems: 32
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - mboxes
> > + - mbox-names
> > + - clocks
> > + - clock-names
> > + - firmware-name
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/imx8mp-clock.h>
> > + dsp_reserved: dsp@92400000 {
> > + reg = <0x92400000 0x1000000>;
> > + no-map;
> > + };
> > + dsp_vdev0vring0: vdev0vring0@942f0000 {
> > + reg = <0x942f0000 0x8000>;
> > + no-map;
> > + };
> > + dsp_vdev0vring1: vdev0vring1@942f8000 {
> > + reg = <0x942f8000 0x8000>;
> > + no-map;
> > + };
> > + dsp_vdev0buffer: vdev0buffer@94300000 {
> > + compatible = "shared-dma-pool";
> > + reg = <0x94300000 0x100000>;
> > + no-map;
> > + };
> > +
> > + dsp: dsp@3b6e8000 {
> > + compatible = "fsl,imx8mp-hifi4";
> > + reg = <0x3B6E8000 0x88000>;
> > + clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_OCRAMA_IPG>,
> > + <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_DSP_ROOT>,
> > + <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_DSPDBG_ROOT>;
> > + clock-names = "dsp_clk1", "dsp_clk2", "dsp_clk3";
> > + firmware-name = "imx/dsp/hifi4.bin";
> > + power-domains = <&audiomix_pd>;
> > + mbox-names = "tx", "rx", "rxdb";
> > + mboxes = <&mu2 0 0>,
> > + <&mu2 1 0>,
> > + <&mu2 3 0>;
> > + memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>,
> > + <&dsp_vdev0vring1>, <&dsp_reserved>;
> > + fsl,dsp-ctrl = <&audio_blk_ctrl>;
> > + };
> > --
> > 2.17.1
> >
> >