diff mbox series

[v4,3/6] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg cpu dai driver

Message ID 1615209750-2357-4-git-send-email-shengjiu.wang@nxp.com (mailing list archive)
State Superseded
Headers show
Series Add audio driver base on rpmsg on i.MX platform | expand

Commit Message

Shengjiu Wang March 8, 2021, 1:22 p.m. UTC
fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
for getting the user's configuration from device tree and configure the
clocks which is used by Cortex-M core. So in this document define the
needed property.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 .../devicetree/bindings/sound/fsl,rpmsg.yaml  | 118 ++++++++++++++++++
 1 file changed, 118 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml

Comments

Rob Herring (Arm) March 10, 2021, 2:48 a.m. UTC | #1
On Mon, Mar 08, 2021 at 09:22:27PM +0800, Shengjiu Wang wrote:
> fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used

Bindings describe h/w blocks, not drivers.

> for getting the user's configuration from device tree and configure the
> clocks which is used by Cortex-M core. So in this document define the
> needed property.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  .../devicetree/bindings/sound/fsl,rpmsg.yaml  | 118 ++++++++++++++++++
>  1 file changed, 118 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> new file mode 100644
> index 000000000000..5731c1fbc0a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> @@ -0,0 +1,118 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP Audio RPMSG CPU DAI Controller
> +
> +maintainers:
> +  - Shengjiu Wang <shengjiu.wang@nxp.com>
> +
> +description: |
> +  fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
> +  touch hardware. It is mainly used for getting the user's configuration
> +  from device tree and configure the clocks which is used by Cortex-M core.
> +  So in this document define the needed property.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx7ulp-rpmsg
> +      - fsl,imx8mn-rpmsg
> +      - fsl,imx8mm-rpmsg
> +      - fsl,imx8mp-rpmsg
> +
> +  model:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: User specified audio sound card name
> +
> +  clocks:
> +    items:
> +      - description: Peripheral clock for register access
> +      - description: Master clock
> +      - description: DMA clock for DMA register access
> +      - description: Parent clock for multiple of 8kHz sample rates
> +      - description: Parent clock for multiple of 11kHz sample rates
> +    minItems: 5

If this doesn't touch hardware, what are these clocks for?

You don't need 'minItems' unless it's less than the number of 'items'.

> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: mclk
> +      - const: dma
> +      - const: pll8k
> +      - const: pll11k
> +    minItems: 5
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  fsl,audioindex:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    default: 0
> +    description: Instance index for sound card in
> +                 M core side, which share one rpmsg
> +                 channel.

We don't do indexes in DT. What's this numbering tied to?

> +
> +  fsl,version:

version of what?

This seems odd at best.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2]

You're going to update this with every new firmware version?

> +    default: 2
> +    description: The version of M core image, which is
> +                 to make driver compatible with different image.
> +
> +  fsl,buffer-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: pre allocate dma buffer size

How can you have DMA, this doesn't touch h/w?

> +
> +  fsl,enable-lpa:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: enable low power audio path.
> +
> +  fsl,rpmsg-out:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      This is a boolean property. If present, the transmitting function
> +      will be enabled.
> +
> +  fsl,rpmsg-in:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      This is a boolean property. If present, the receiving function
> +      will be enabled.
> +
> +  fsl,codec-type:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2]
> +    default: 0
> +    description: Sometimes the codec is registered by
> +                 driver not by the device tree, this items
> +                 can be used to distinguish codecs.

How does one decide what value to use?

> +
> +  audio-codec:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: The phandle of the audio codec

The codec is controlled from the Linux side?

> +
> +  memory-region:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the reserved memory nodes
> +
> +required:
> +  - compatible
> +  - fsl,audioindex
> +  - fsl,version
> +  - fsl,buffer-size
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    rpmsg_audio: rpmsg_audio {
> +        compatible = "fsl,imx8mn-rpmsg";
> +        fsl,audioindex = <0> ;
> +        fsl,version = <2>;
> +        fsl,buffer-size = <0x6000000>;
> +        fsl,enable-lpa;

How does this work? Don't you need somewhere to put the 'rpmsg' data?

> +    };
> -- 
> 2.27.0
>
Shengjiu Wang March 10, 2021, 1:33 p.m. UTC | #2
Hi Rob

On Wed, Mar 10, 2021 at 10:49 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Mar 08, 2021 at 09:22:27PM +0800, Shengjiu Wang wrote:
> > fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
>
> Bindings describe h/w blocks, not drivers.

I will modify the descriptions. but here it is a virtual device.  the
mapping in real h/w is cortex M core, Cortex M core controls the SAI,
DMA interface. What we see from Linux side is a audio service
through rpmsg channel.

>
> > for getting the user's configuration from device tree and configure the
> > clocks which is used by Cortex-M core. So in this document define the
> > needed property.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  .../devicetree/bindings/sound/fsl,rpmsg.yaml  | 118 ++++++++++++++++++
> >  1 file changed, 118 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > new file mode 100644
> > index 000000000000..5731c1fbc0a6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > @@ -0,0 +1,118 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP Audio RPMSG CPU DAI Controller
> > +
> > +maintainers:
> > +  - Shengjiu Wang <shengjiu.wang@nxp.com>
> > +
> > +description: |
> > +  fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
> > +  touch hardware. It is mainly used for getting the user's configuration
> > +  from device tree and configure the clocks which is used by Cortex-M core.
> > +  So in this document define the needed property.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,imx7ulp-rpmsg
> > +      - fsl,imx8mn-rpmsg
> > +      - fsl,imx8mm-rpmsg
> > +      - fsl,imx8mp-rpmsg
> > +
> > +  model:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description: User specified audio sound card name
> > +
> > +  clocks:
> > +    items:
> > +      - description: Peripheral clock for register access
> > +      - description: Master clock
> > +      - description: DMA clock for DMA register access
> > +      - description: Parent clock for multiple of 8kHz sample rates
> > +      - description: Parent clock for multiple of 11kHz sample rates
> > +    minItems: 5
>
> If this doesn't touch hardware, what are these clocks for?

When the cortex-M core support audio service, these clock
needed prepared & enabled by ALSA driver.

>
> You don't need 'minItems' unless it's less than the number of 'items'.

Ok, I will remove this minItems.

>
> > +
> > +  clock-names:
> > +    items:
> > +      - const: ipg
> > +      - const: mclk
> > +      - const: dma
> > +      - const: pll8k
> > +      - const: pll11k
> > +    minItems: 5
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  fsl,audioindex:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1]
> > +    default: 0
> > +    description: Instance index for sound card in
> > +                 M core side, which share one rpmsg
> > +                 channel.
>
> We don't do indexes in DT. What's this numbering tied to?

I will remove it. it is not necessary

>
> > +
> > +  fsl,version:
>
> version of what?
>
> This seems odd at best.
>

I will remove it.  it is not necessary

> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [1, 2]
>
> You're going to update this with every new firmware version?
>
> > +    default: 2
> > +    description: The version of M core image, which is
> > +                 to make driver compatible with different image.
> > +
> > +  fsl,buffer-size:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: pre allocate dma buffer size
>
> How can you have DMA, this doesn't touch h/w?

The DMA is handled by M core image for sound playback
and capture. we need to allocated buffer in Linux side.
here just make the buffer size to be configurable.
>
> > +
> > +  fsl,enable-lpa:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description: enable low power audio path.
> > +
> > +  fsl,rpmsg-out:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description: |
> > +      This is a boolean property. If present, the transmitting function
> > +      will be enabled.
> > +
> > +  fsl,rpmsg-in:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description: |
> > +      This is a boolean property. If present, the receiving function
> > +      will be enabled.
> > +
> > +  fsl,codec-type:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1, 2]
> > +    default: 0
> > +    description: Sometimes the codec is registered by
> > +                 driver not by the device tree, this items
> > +                 can be used to distinguish codecs.
>
> How does one decide what value to use?

I will add more description:
0: dummy codec
1: WM8960 codec
2: AK4497 codec

>
> > +
> > +  audio-codec:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: The phandle of the audio codec
>
> The codec is controlled from the Linux side?

yes.

>
> > +
> > +  memory-region:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the reserved memory nodes
> > +
> > +required:
> > +  - compatible
> > +  - fsl,audioindex
> > +  - fsl,version
> > +  - fsl,buffer-size
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    rpmsg_audio: rpmsg_audio {
> > +        compatible = "fsl,imx8mn-rpmsg";
> > +        fsl,audioindex = <0> ;
> > +        fsl,version = <2>;
> > +        fsl,buffer-size = <0x6000000>;
> > +        fsl,enable-lpa;
>
> How does this work? Don't you need somewhere to put the 'rpmsg' data?

The rpmsg data is not handled in this "rpmsg_audio" device, it is just to
prepare the resource for rpmsg audio function, the clock, the memory
the power...

The rpmsg data is handled in imx-pcm-rpmsg.c and imx-audio-rpmsg.c
These devices is registered by imx remoteproc driver.


I will update this document in v5

Best regards
Wang Shengjiu
Rob Herring (Arm) March 10, 2021, 9:12 p.m. UTC | #3
On Wed, Mar 10, 2021 at 6:33 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
>
> Hi Rob
>
> On Wed, Mar 10, 2021 at 10:49 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Mar 08, 2021 at 09:22:27PM +0800, Shengjiu Wang wrote:
> > > fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
> >
> > Bindings describe h/w blocks, not drivers.
>
> I will modify the descriptions. but here it is a virtual device.  the
> mapping in real h/w is cortex M core, Cortex M core controls the SAI,
> DMA interface. What we see from Linux side is a audio service
> through rpmsg channel.

It's describing the h/w from the view of the OS. It's not important
that it's a Cortex-M, but how you interface to it whether that's
shared registers, mailbox, etc. And it's what resources the block uses
that the OS controls.

> > > for getting the user's configuration from device tree and configure the
> > > clocks which is used by Cortex-M core. So in this document define the
> > > needed property.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > >  .../devicetree/bindings/sound/fsl,rpmsg.yaml  | 118 ++++++++++++++++++
> > >  1 file changed, 118 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > new file mode 100644
> > > index 000000000000..5731c1fbc0a6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > @@ -0,0 +1,118 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NXP Audio RPMSG CPU DAI Controller
> > > +
> > > +maintainers:
> > > +  - Shengjiu Wang <shengjiu.wang@nxp.com>
> > > +
> > > +description: |
> > > +  fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
> > > +  touch hardware. It is mainly used for getting the user's configuration
> > > +  from device tree and configure the clocks which is used by Cortex-M core.
> > > +  So in this document define the needed property.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - fsl,imx7ulp-rpmsg
> > > +      - fsl,imx8mn-rpmsg
> > > +      - fsl,imx8mm-rpmsg
> > > +      - fsl,imx8mp-rpmsg
> > > +
> > > +  model:
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    description: User specified audio sound card name
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: Peripheral clock for register access
> > > +      - description: Master clock
> > > +      - description: DMA clock for DMA register access
> > > +      - description: Parent clock for multiple of 8kHz sample rates
> > > +      - description: Parent clock for multiple of 11kHz sample rates
> > > +    minItems: 5
> >
> > If this doesn't touch hardware, what are these clocks for?
>
> When the cortex-M core support audio service, these clock
> needed prepared & enabled by ALSA driver.
>
> >
> > You don't need 'minItems' unless it's less than the number of 'items'.
>
> Ok, I will remove this minItems.
>
> >
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: ipg
> > > +      - const: mclk
> > > +      - const: dma
> > > +      - const: pll8k
> > > +      - const: pll11k
> > > +    minItems: 5
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  fsl,audioindex:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [0, 1]
> > > +    default: 0
> > > +    description: Instance index for sound card in
> > > +                 M core side, which share one rpmsg
> > > +                 channel.
> >
> > We don't do indexes in DT. What's this numbering tied to?
>
> I will remove it. it is not necessary
>
> >
> > > +
> > > +  fsl,version:
> >
> > version of what?
> >
> > This seems odd at best.
> >
>
> I will remove it.  it is not necessary
>
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [1, 2]
> >
> > You're going to update this with every new firmware version?
> >
> > > +    default: 2
> > > +    description: The version of M core image, which is
> > > +                 to make driver compatible with different image.
> > > +
> > > +  fsl,buffer-size:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: pre allocate dma buffer size
> >
> > How can you have DMA, this doesn't touch h/w?
>
> The DMA is handled by M core image for sound playback
> and capture. we need to allocated buffer in Linux side.
> here just make the buffer size to be configurable.

Do we set audio buffer sizes for other audio devices in DT? If not,
why is this special? If so, why is it not common.

> > > +  fsl,enable-lpa:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > +    description: enable low power audio path.
> > > +
> > > +  fsl,rpmsg-out:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > +    description: |
> > > +      This is a boolean property. If present, the transmitting function
> > > +      will be enabled.
> > > +
> > > +  fsl,rpmsg-in:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > +    description: |
> > > +      This is a boolean property. If present, the receiving function
> > > +      will be enabled.
> > > +
> > > +  fsl,codec-type:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [0, 1, 2]
> > > +    default: 0
> > > +    description: Sometimes the codec is registered by
> > > +                 driver not by the device tree, this items
> > > +                 can be used to distinguish codecs.
> >
> > How does one decide what value to use?
>
> I will add more description:
> 0: dummy codec
> 1: WM8960 codec
> 2: AK4497 codec

I assume the last 2 cases have nodes in DT (pointed to by
'audio-codec'), so this is redundant.

> > > +
> > > +  audio-codec:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description: The phandle of the audio codec
> >
> > The codec is controlled from the Linux side?
>
> yes.
>
> >
> > > +
> > > +  memory-region:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description: phandle to the reserved memory nodes
> > > +
> > > +required:
> > > +  - compatible
> > > +  - fsl,audioindex
> > > +  - fsl,version
> > > +  - fsl,buffer-size
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    rpmsg_audio: rpmsg_audio {
> > > +        compatible = "fsl,imx8mn-rpmsg";
> > > +        fsl,audioindex = <0> ;
> > > +        fsl,version = <2>;
> > > +        fsl,buffer-size = <0x6000000>;
> > > +        fsl,enable-lpa;
> >
> > How does this work? Don't you need somewhere to put the 'rpmsg' data?
>
> The rpmsg data is not handled in this "rpmsg_audio" device, it is just to
> prepare the resource for rpmsg audio function, the clock, the memory
> the power...
>
> The rpmsg data is handled in imx-pcm-rpmsg.c and imx-audio-rpmsg.c
> These devices is registered by imx remoteproc driver.

Then what is 'memory-region' for? You need that, a mailbox, or ???
somewhere in DT.

Rob
Shengjiu Wang March 11, 2021, 10:58 a.m. UTC | #4
Hi Rob

On Thu, Mar 11, 2021 at 5:12 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Mar 10, 2021 at 6:33 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
> >
> > Hi Rob
> >
> > On Wed, Mar 10, 2021 at 10:49 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Mon, Mar 08, 2021 at 09:22:27PM +0800, Shengjiu Wang wrote:
> > > > fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
> > >
> > > Bindings describe h/w blocks, not drivers.
> >
> > I will modify the descriptions. but here it is a virtual device.  the
> > mapping in real h/w is cortex M core, Cortex M core controls the SAI,
> > DMA interface. What we see from Linux side is a audio service
> > through rpmsg channel.
>
> It's describing the h/w from the view of the OS. It's not important
> that it's a Cortex-M, but how you interface to it whether that's
> shared registers, mailbox, etc. And it's what resources the block uses
> that the OS controls.

ok.

>
> > > > for getting the user's configuration from device tree and configure the
> > > > clocks which is used by Cortex-M core. So in this document define the
> > > > needed property.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/sound/fsl,rpmsg.yaml  | 118 ++++++++++++++++++
> > > >  1 file changed, 118 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > > new file mode 100644
> > > > index 000000000000..5731c1fbc0a6
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > > @@ -0,0 +1,118 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: NXP Audio RPMSG CPU DAI Controller
> > > > +
> > > > +maintainers:
> > > > +  - Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > +
> > > > +description: |
> > > > +  fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
> > > > +  touch hardware. It is mainly used for getting the user's configuration
> > > > +  from device tree and configure the clocks which is used by Cortex-M core.
> > > > +  So in this document define the needed property.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - fsl,imx7ulp-rpmsg
> > > > +      - fsl,imx8mn-rpmsg
> > > > +      - fsl,imx8mm-rpmsg
> > > > +      - fsl,imx8mp-rpmsg
> > > > +
> > > > +  model:
> > > > +    $ref: /schemas/types.yaml#/definitions/string
> > > > +    description: User specified audio sound card name
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: Peripheral clock for register access
> > > > +      - description: Master clock
> > > > +      - description: DMA clock for DMA register access
> > > > +      - description: Parent clock for multiple of 8kHz sample rates
> > > > +      - description: Parent clock for multiple of 11kHz sample rates
> > > > +    minItems: 5
> > >
> > > If this doesn't touch hardware, what are these clocks for?
> >
> > When the cortex-M core support audio service, these clock
> > needed prepared & enabled by ALSA driver.
> >
> > >
> > > You don't need 'minItems' unless it's less than the number of 'items'.
> >
> > Ok, I will remove this minItems.
> >
> > >
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: ipg
> > > > +      - const: mclk
> > > > +      - const: dma
> > > > +      - const: pll8k
> > > > +      - const: pll11k
> > > > +    minItems: 5
> > > > +
> > > > +  power-domains:
> > > > +    maxItems: 1
> > > > +
> > > > +  fsl,audioindex:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [0, 1]
> > > > +    default: 0
> > > > +    description: Instance index for sound card in
> > > > +                 M core side, which share one rpmsg
> > > > +                 channel.
> > >
> > > We don't do indexes in DT. What's this numbering tied to?
> >
> > I will remove it. it is not necessary
> >
> > >
> > > > +
> > > > +  fsl,version:
> > >
> > > version of what?
> > >
> > > This seems odd at best.
> > >
> >
> > I will remove it.  it is not necessary
> >
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [1, 2]
> > >
> > > You're going to update this with every new firmware version?
> > >
> > > > +    default: 2
> > > > +    description: The version of M core image, which is
> > > > +                 to make driver compatible with different image.
> > > > +
> > > > +  fsl,buffer-size:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: pre allocate dma buffer size
> > >
> > > How can you have DMA, this doesn't touch h/w?
> >
> > The DMA is handled by M core image for sound playback
> > and capture. we need to allocated buffer in Linux side.
> > here just make the buffer size to be configurable.
>
> Do we set audio buffer sizes for other audio devices in DT? If not,
> why is this special? If so, why is it not common.

No. I will move it to driver.

>
> > > > +  fsl,enable-lpa:
> > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > +    description: enable low power audio path.
> > > > +
> > > > +  fsl,rpmsg-out:
> > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > +    description: |
> > > > +      This is a boolean property. If present, the transmitting function
> > > > +      will be enabled.
> > > > +
> > > > +  fsl,rpmsg-in:
> > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > +    description: |
> > > > +      This is a boolean property. If present, the receiving function
> > > > +      will be enabled.
> > > > +
> > > > +  fsl,codec-type:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [0, 1, 2]
> > > > +    default: 0
> > > > +    description: Sometimes the codec is registered by
> > > > +                 driver not by the device tree, this items
> > > > +                 can be used to distinguish codecs.
> > >
> > > How does one decide what value to use?
> >
> > I will add more description:
> > 0: dummy codec
> > 1: WM8960 codec
> > 2: AK4497 codec
>
> I assume the last 2 cases have nodes in DT (pointed to by
> 'audio-codec'), so this is redundant.

Ok, will remove it.

>
> > > > +
> > > > +  audio-codec:
> > > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > > +    description: The phandle of the audio codec
> > >
> > > The codec is controlled from the Linux side?
> >
> > yes.
> >
> > >
> > > > +
> > > > +  memory-region:
> > > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > > +    description: phandle to the reserved memory nodes
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - fsl,audioindex
> > > > +  - fsl,version
> > > > +  - fsl,buffer-size
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    rpmsg_audio: rpmsg_audio {
> > > > +        compatible = "fsl,imx8mn-rpmsg";
> > > > +        fsl,audioindex = <0> ;
> > > > +        fsl,version = <2>;
> > > > +        fsl,buffer-size = <0x6000000>;
> > > > +        fsl,enable-lpa;
> > >
> > > How does this work? Don't you need somewhere to put the 'rpmsg' data?
> >
> > The rpmsg data is not handled in this "rpmsg_audio" device, it is just to
> > prepare the resource for rpmsg audio function, the clock, the memory
> > the power...
> >
> > The rpmsg data is handled in imx-pcm-rpmsg.c and imx-audio-rpmsg.c
> > These devices is registered by imx remoteproc driver.
>
> Then what is 'memory-region' for? You need that, a mailbox, or ???
> somewhere in DT.
>
The M core can't access all the DDR memory space on some platform,
so use 'memory-region' reserve a specific memory for dma buffer
which M core can access.

best regards
wang shengjiu
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
new file mode 100644
index 000000000000..5731c1fbc0a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
@@ -0,0 +1,118 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP Audio RPMSG CPU DAI Controller
+
+maintainers:
+  - Shengjiu Wang <shengjiu.wang@nxp.com>
+
+description: |
+  fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
+  touch hardware. It is mainly used for getting the user's configuration
+  from device tree and configure the clocks which is used by Cortex-M core.
+  So in this document define the needed property.
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx7ulp-rpmsg
+      - fsl,imx8mn-rpmsg
+      - fsl,imx8mm-rpmsg
+      - fsl,imx8mp-rpmsg
+
+  model:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: User specified audio sound card name
+
+  clocks:
+    items:
+      - description: Peripheral clock for register access
+      - description: Master clock
+      - description: DMA clock for DMA register access
+      - description: Parent clock for multiple of 8kHz sample rates
+      - description: Parent clock for multiple of 11kHz sample rates
+    minItems: 5
+
+  clock-names:
+    items:
+      - const: ipg
+      - const: mclk
+      - const: dma
+      - const: pll8k
+      - const: pll11k
+    minItems: 5
+
+  power-domains:
+    maxItems: 1
+
+  fsl,audioindex:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    default: 0
+    description: Instance index for sound card in
+                 M core side, which share one rpmsg
+                 channel.
+
+  fsl,version:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+    default: 2
+    description: The version of M core image, which is
+                 to make driver compatible with different image.
+
+  fsl,buffer-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: pre allocate dma buffer size
+
+  fsl,enable-lpa:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: enable low power audio path.
+
+  fsl,rpmsg-out:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: |
+      This is a boolean property. If present, the transmitting function
+      will be enabled.
+
+  fsl,rpmsg-in:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: |
+      This is a boolean property. If present, the receiving function
+      will be enabled.
+
+  fsl,codec-type:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2]
+    default: 0
+    description: Sometimes the codec is registered by
+                 driver not by the device tree, this items
+                 can be used to distinguish codecs.
+
+  audio-codec:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: The phandle of the audio codec
+
+  memory-region:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the reserved memory nodes
+
+required:
+  - compatible
+  - fsl,audioindex
+  - fsl,version
+  - fsl,buffer-size
+
+additionalProperties: false
+
+examples:
+  - |
+    rpmsg_audio: rpmsg_audio {
+        compatible = "fsl,imx8mn-rpmsg";
+        fsl,audioindex = <0> ;
+        fsl,version = <2>;
+        fsl,buffer-size = <0x6000000>;
+        fsl,enable-lpa;
+    };