Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

Thanks for the review,

On 01/03/18 20:34, Mark Brown wrote:

On Thu, Feb 22, 2018 at 10:03:42AM +, Srinivas Kandagatla wrote:

On 22/02/18 00:14, Rob Herring wrote:



Am I saying a single DT node for this? Yes, perhaps.

Yes, I will give that a go.

So we will endup having something like this in DT for one frontend and
backend dailink:


Let's not start encoding DPCM into DT, DPCM is very much an
implemntation detail of the current stack which we're gradually pushing
towards replacing with something better.  What we want to be doing is
just treating components inside the SoC the same as components in a
CODEC, the routing within a SoC being the same as in a CODEC and
similarly for externally connected devices.


fe@1 {
is-fe;
 link-name = "MultiMedia1";


In particular having the concept of "front end" in the DT is *very*
implementation specific.  We should just be describing the connections
that exist in the system.
Yep, it makes sense. is-fe flag will be removed in next version, we can 
determine this based on codec dai, so from DT side FE or BE should not 
be any different.


Thanks,
srini

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

Thanks for the review,

On 01/03/18 20:34, Mark Brown wrote:

On Thu, Feb 22, 2018 at 10:03:42AM +, Srinivas Kandagatla wrote:

On 22/02/18 00:14, Rob Herring wrote:



Am I saying a single DT node for this? Yes, perhaps.

Yes, I will give that a go.

So we will endup having something like this in DT for one frontend and
backend dailink:


Let's not start encoding DPCM into DT, DPCM is very much an
implemntation detail of the current stack which we're gradually pushing
towards replacing with something better.  What we want to be doing is
just treating components inside the SoC the same as components in a
CODEC, the routing within a SoC being the same as in a CODEC and
similarly for externally connected devices.


fe@1 {
is-fe;
 link-name = "MultiMedia1";


In particular having the concept of "front end" in the DT is *very*
implementation specific.  We should just be describing the connections
that exist in the system.
Yep, it makes sense. is-fe flag will be removed in next version, we can 
determine this based on codec dai, so from DT side FE or BE should not 
be any different.


Thanks,
srini

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Mark Brown]

On Thu, Feb 22, 2018 at 10:03:42AM +, Srinivas Kandagatla wrote:
> On 22/02/18 00:14, Rob Herring wrote:

> > Am I saying a single DT node for this? Yes, perhaps.
> Yes, I will give that a go.
> 
> So we will endup having something like this in DT for one frontend and
> backend dailink:

Let's not start encoding DPCM into DT, DPCM is very much an
implemntation detail of the current stack which we're gradually pushing
towards replacing with something better.  What we want to be doing is
just treating components inside the SoC the same as components in a
CODEC, the routing within a SoC being the same as in a CODEC and
similarly for externally connected devices.

>   fe@1 {
>   is-fe;
> link-name = "MultiMedia1";

In particular having the concept of "front end" in the DT is *very*
implementation specific.  We should just be describing the connections
that exist in the system.


signature.asc
Description: PGP signature

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Mark Brown]

On Thu, Feb 22, 2018 at 10:03:42AM +, Srinivas Kandagatla wrote:
> On 22/02/18 00:14, Rob Herring wrote:

> > Am I saying a single DT node for this? Yes, perhaps.
> Yes, I will give that a go.
> 
> So we will endup having something like this in DT for one frontend and
> backend dailink:

Let's not start encoding DPCM into DT, DPCM is very much an
implemntation detail of the current stack which we're gradually pushing
towards replacing with something better.  What we want to be doing is
just treating components inside the SoC the same as components in a
CODEC, the routing within a SoC being the same as in a CODEC and
similarly for externally connected devices.

>   fe@1 {
>   is-fe;
> link-name = "MultiMedia1";

In particular having the concept of "front end" in the DT is *very*
implementation specific.  We should just be describing the connections
that exist in the system.


signature.asc
Description: PGP signature

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

On 22/02/18 10:03, Srinivas Kandagatla wrote:

Also the versions of each service are independent to each other.


Not sure I follow the last statement. Meaning firmware updates change
the services?
Sorry for not being clear, so the services like AFE, ASM, ADM have 
different version numbers for a given SoC/firmware.


Not 100% sure if firmware updates would change the service version 
number, even if it does, it can be queried dynamically on new B family 
SoCs, and on older A Family SoCs I have not seen the firmware updates in 
last 4 years.



I don't see any versioning of services here.


Yes, Plan is that it will be part of the compatible string in cases 
where version query is not supported on older QCOM A family SoCs.

On B Family SoCs we can query the version dynamically.







apr {
  compatible = "qcom,apr-v2";
  qcom,smd-channels = "apr_audio_svc";
  qcom,apr-dest-domain-id = ;

  apr-services {
  q6core {
  qcom,apr-svc-name = "CORE";
  qcom,apr-svc-id = ;
  compatible = "qcom,q6core";
  };

  q6afe: q6afe {
  compatible = "qcom,q6afe";
  qcom,apr-svc-name = "AFE";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6asm: q6asm {
  compatible = "qcom,q6asm";
  qcom,apr-svc-name = "ASM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6adm: q6adm {, "external-sleep"
  compatible = "qcom,q6adm";
  qcom,apr-svc-name = "ADM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <0>;
  };



All these DAI nodes could be a single node and the cell value be the
svc-id?


So we will have 2 cell values, one representing the apr service and 
other the dai.



No, DAI's here are both backends and frontends, and some of the services
like core, USM are not DAI's

Are you also saying that we should have a single driver entity for 
all these

services?


DT nodes do not equate driver entities. A driver can instantiate other 
drivers.


Am I saying a single DT node for this? Yes, perhaps.

Yes, I will give that a go.


I did try your suggestion of making audio services into a single node 
and it ended up more messy and non scalable.


Mainly because q6afe node has more than one interface type of backend 
dais (child nodes).


Each of this backend dai may need board specific setting ex: MI2S case 
where board could wire up specific tx and rx lines from 4 possible lines 
for that board. Also other type of dais also have some interface 
specific properties.


MI2S example:
apr {
...
q6afe: q6afe {
compatible = "qcom,q6afe";
qcom,apr-svc-name = "AFE";
qcom,apr-svc-id = ;
pinctrl-0 = <_sec_tlmm_lines_act>;
pinctrl-names = "default";
#sound-dai-cells = <1>;

mi2s_prim_rx_dai@34{
reg = <34>;
rx-lines = <0>;
...
};

mi2s_prim_tx_dai@35{
reg = <35>;
tx-lines = <1 2>
...
};
...
};
};

Here is block diagram to give a quick overview of the components


  +-+  +-+ +-+
  |  q6asm  |  |q6routing| | q6afe   |
  |  fedai  | <--> |  mixers | <-> | bedai   |
  +-+  +-+ +-+
  ^ ^   ^
  | |   |
  |  +--++  |
  |  |  ||  |
  v  v  vv  v
  +-+  +-+ +-+
  |   q6ASM |  |  q6ADM  | |   q6AFE |
  +-+  +-+ +-+
  ^ ^   ^  ^
  | |   | CPU Side |
--+-+---+
  | |   |
  |APR  |APR|APR
  | |   |
  |  +--++  |
  |  |  ||  |
+-+--+---+--+---
  |  |  ||  | QDSP Side |
  v  v  vv  v   v
 +-+  +-+ +-+
 |   ASM   | <--> |   ADM   | 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

On 22/02/18 10:03, Srinivas Kandagatla wrote:

Also the versions of each service are independent to each other.


Not sure I follow the last statement. Meaning firmware updates change
the services?
Sorry for not being clear, so the services like AFE, ASM, ADM have 
different version numbers for a given SoC/firmware.


Not 100% sure if firmware updates would change the service version 
number, even if it does, it can be queried dynamically on new B family 
SoCs, and on older A Family SoCs I have not seen the firmware updates in 
last 4 years.



I don't see any versioning of services here.


Yes, Plan is that it will be part of the compatible string in cases 
where version query is not supported on older QCOM A family SoCs.

On B Family SoCs we can query the version dynamically.







apr {
  compatible = "qcom,apr-v2";
  qcom,smd-channels = "apr_audio_svc";
  qcom,apr-dest-domain-id = ;

  apr-services {
  q6core {
  qcom,apr-svc-name = "CORE";
  qcom,apr-svc-id = ;
  compatible = "qcom,q6core";
  };

  q6afe: q6afe {
  compatible = "qcom,q6afe";
  qcom,apr-svc-name = "AFE";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6asm: q6asm {
  compatible = "qcom,q6asm";
  qcom,apr-svc-name = "ASM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6adm: q6adm {, "external-sleep"
  compatible = "qcom,q6adm";
  qcom,apr-svc-name = "ADM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <0>;
  };



All these DAI nodes could be a single node and the cell value be the
svc-id?


So we will have 2 cell values, one representing the apr service and 
other the dai.



No, DAI's here are both backends and frontends, and some of the services
like core, USM are not DAI's

Are you also saying that we should have a single driver entity for 
all these

services?


DT nodes do not equate driver entities. A driver can instantiate other 
drivers.


Am I saying a single DT node for this? Yes, perhaps.

Yes, I will give that a go.


I did try your suggestion of making audio services into a single node 
and it ended up more messy and non scalable.


Mainly because q6afe node has more than one interface type of backend 
dais (child nodes).


Each of this backend dai may need board specific setting ex: MI2S case 
where board could wire up specific tx and rx lines from 4 possible lines 
for that board. Also other type of dais also have some interface 
specific properties.


MI2S example:
apr {
...
q6afe: q6afe {
compatible = "qcom,q6afe";
qcom,apr-svc-name = "AFE";
qcom,apr-svc-id = ;
pinctrl-0 = <_sec_tlmm_lines_act>;
pinctrl-names = "default";
#sound-dai-cells = <1>;

mi2s_prim_rx_dai@34{
reg = <34>;
rx-lines = <0>;
...
};

mi2s_prim_tx_dai@35{
reg = <35>;
tx-lines = <1 2>
...
};
...
};
};

Here is block diagram to give a quick overview of the components


  +-+  +-+ +-+
  |  q6asm  |  |q6routing| | q6afe   |
  |  fedai  | <--> |  mixers | <-> | bedai   |
  +-+  +-+ +-+
  ^ ^   ^
  | |   |
  |  +--++  |
  |  |  ||  |
  v  v  vv  v
  +-+  +-+ +-+
  |   q6ASM |  |  q6ADM  | |   q6AFE |
  +-+  +-+ +-+
  ^ ^   ^  ^
  | |   | CPU Side |
--+-+---+
  | |   |
  |APR  |APR|APR
  | |   |
  |  +--++  |
  |  |  ||  |
+-+--+---+--+---
  |  |  ||  | QDSP Side |
  v  v  vv  v   v
 +-+  +-+ +-+
 |   ASM   | <--> |   ADM   | 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

On 22/02/18 00:14, Rob Herring wrote:

On Tue, Feb 20, 2018 at 3:33 AM, Srinivas Kandagatla
 wrote:

Thanks for your review comments,


On 18/02/18 23:04, Rob Herring wrote:


On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:


Thanks for the review,

On 13/02/18 23:12, Rob Herring wrote:


On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org
wrote:


From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet
Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
.../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83
++
1 file changed, 83 insertions(+)
create mode 100644
Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+

...



It's not a good design generally to mix different types of nodes at one
level.



I agree, may be I can split the services and devices into different
subnodes
like below, which should avoid mixing different types of nodes.

Does this sound good to you?



Seems your original example wasn't so complete...


Yep, I will fix it in next version.


I don't see why you need all these nodes in the first place. For a
single SoC, how much does all this vary?


It might not vary for a given SoC, but It does vary across the SoCs.
Also the versions of each service are independent to each other.


Not sure I follow the last statement. Meaning firmware updates change
the services?
Sorry for not being clear, so the services like AFE, ASM, ADM have 
different version numbers for a given SoC/firmware.


Not 100% sure if firmware updates would change the service version 
number, even if it does, it can be queried dynamically on new B family 
SoCs, and on older A Family SoCs I have not seen the firmware updates in 
last 4 years.



I don't see any versioning of services here.


Yes, Plan is that it will be part of the compatible string in cases 
where version query is not supported on older QCOM A family SoCs.

On B Family SoCs we can query the version dynamically.







apr {
  compatible = "qcom,apr-v2";
  qcom,smd-channels = "apr_audio_svc";
  qcom,apr-dest-domain-id = ;

  apr-services {
  q6core {
  qcom,apr-svc-name = "CORE";
  qcom,apr-svc-id = ;
  compatible = "qcom,q6core";
  };

  q6afe: q6afe {
  compatible = "qcom,q6afe";
  qcom,apr-svc-name = "AFE";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6asm: q6asm {
  compatible = "qcom,q6asm";
  qcom,apr-svc-name = "ASM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6adm: q6adm {
  compatible = "qcom,q6adm";
  qcom,apr-svc-name = "ADM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <0>;
  };



All these DAI nodes could be a single node and the cell value be the
svc-id?


So we will have 2 cell values, one representing the apr service and 
other the dai.



No, DAI's here are both backends and frontends, and some of the services
like core, USM are not DAI's

Are you also saying that we should have a single driver entity for all these
services?


DT nodes do not equate driver entities. A driver can instantiate other drivers.

Am I saying a single DT node for this? Yes, perhaps.

Yes, I will give that a go.

So we will endup having something like this in DT for one frontend and 
backend dailink:


apr {
compatible = "qcom,apr-v2";
qcom,smd-channels = "apr_audio_svc";
qcom,apr-dest-domain-id = 

q6audio: audiosvc {
compatible = "qcom,q6audio-svc";
qcom,apr-svcs = ;
qcom,apr-svc-names = "CORE", "AFE", "ASM";
};

sndcard {
compatible = "qcom,msm8996-snd-card"
qcom,model = "DB820c";
qcom,audio-routing =
"RX_BIAS", "MCLK";
fe@1 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

On 22/02/18 00:14, Rob Herring wrote:

On Tue, Feb 20, 2018 at 3:33 AM, Srinivas Kandagatla
 wrote:

Thanks for your review comments,


On 18/02/18 23:04, Rob Herring wrote:


On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:


Thanks for the review,

On 13/02/18 23:12, Rob Herring wrote:


On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org
wrote:


From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet
Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
.../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83
++
1 file changed, 83 insertions(+)
create mode 100644
Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+

...



It's not a good design generally to mix different types of nodes at one
level.



I agree, may be I can split the services and devices into different
subnodes
like below, which should avoid mixing different types of nodes.

Does this sound good to you?



Seems your original example wasn't so complete...


Yep, I will fix it in next version.


I don't see why you need all these nodes in the first place. For a
single SoC, how much does all this vary?


It might not vary for a given SoC, but It does vary across the SoCs.
Also the versions of each service are independent to each other.


Not sure I follow the last statement. Meaning firmware updates change
the services?
Sorry for not being clear, so the services like AFE, ASM, ADM have 
different version numbers for a given SoC/firmware.


Not 100% sure if firmware updates would change the service version 
number, even if it does, it can be queried dynamically on new B family 
SoCs, and on older A Family SoCs I have not seen the firmware updates in 
last 4 years.



I don't see any versioning of services here.


Yes, Plan is that it will be part of the compatible string in cases 
where version query is not supported on older QCOM A family SoCs.

On B Family SoCs we can query the version dynamically.







apr {
  compatible = "qcom,apr-v2";
  qcom,smd-channels = "apr_audio_svc";
  qcom,apr-dest-domain-id = ;

  apr-services {
  q6core {
  qcom,apr-svc-name = "CORE";
  qcom,apr-svc-id = ;
  compatible = "qcom,q6core";
  };

  q6afe: q6afe {
  compatible = "qcom,q6afe";
  qcom,apr-svc-name = "AFE";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6asm: q6asm {
  compatible = "qcom,q6asm";
  qcom,apr-svc-name = "ASM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <1>;
  };

  q6adm: q6adm {
  compatible = "qcom,q6adm";
  qcom,apr-svc-name = "ADM";
  qcom,apr-svc-id = ;
  #sound-dai-cells = <0>;
  };



All these DAI nodes could be a single node and the cell value be the
svc-id?


So we will have 2 cell values, one representing the apr service and 
other the dai.



No, DAI's here are both backends and frontends, and some of the services
like core, USM are not DAI's

Are you also saying that we should have a single driver entity for all these
services?


DT nodes do not equate driver entities. A driver can instantiate other drivers.

Am I saying a single DT node for this? Yes, perhaps.

Yes, I will give that a go.

So we will endup having something like this in DT for one frontend and 
backend dailink:


apr {
compatible = "qcom,apr-v2";
qcom,smd-channels = "apr_audio_svc";
qcom,apr-dest-domain-id = 

q6audio: audiosvc {
compatible = "qcom,q6audio-svc";
qcom,apr-svcs = ;
qcom,apr-svc-names = "CORE", "AFE", "ASM";
};

sndcard {
compatible = "qcom,msm8996-snd-card"
qcom,model = "DB820c";
qcom,audio-routing =
"RX_BIAS", "MCLK";
fe@1 {
is-fe;
link-name = "MultiMedia1";
 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Rob Herring]

On Tue, Feb 20, 2018 at 3:33 AM, Srinivas Kandagatla
 wrote:
> Thanks for your review comments,
>
>
> On 18/02/18 23:04, Rob Herring wrote:
>>
>> On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:
>>>
>>> Thanks for the review,
>>>
>>> On 13/02/18 23:12, Rob Herring wrote:

 On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org
 wrote:
>
> From: Srinivas Kandagatla 
>
> This patch add dt bindings for Qualcomm APR (Asynchronous Packet
> Router)
> bus driver. This bus is used for communicating with DSP which provides
> audio and various other services to cpu.
>
> Signed-off-by: Srinivas Kandagatla 
> ---
>.../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83
> ++
>1 file changed, 83 insertions(+)
>create mode 100644
> Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> new file mode 100644
> index ..1b95fbfed348
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> @@ -0,0 +1,83 @@
> +Qualcomm APR (Asynchronous Packet Router) binding
> +
> +This binding describes the Qualcomm APR. APR is a IPC protocol for
> +communication between Application processor and QDSP. APR is mainly
> +used for audio/voice services on the QDSP.
> +
> +- compatible:
> +   Usage: required
> +   Value type: 
> +   Definition: must be "qcom,apr-v", example
> "qcom,apr-v2"
> +
> +- qcom,apr-dest-domain-id
> +   Usage: required
> +   Value type: 
> +   Definition: Destination processor ID.
> +   Possible values are :
> +   1 - APR simulator
> +   2 - PC
> +   3 - MODEM
> +   4 - ADSP
> +   5 - APPS
> +   6 - MODEM2
> +   7 - APPS2
> +
> += APR SERVICES
> +Each subnode of the APR node can represent service tied to this apr.
> The name
> +of the nodes are not important. The properties of these nodes are
> defined
> +by the individual bindings for the specific service
> +- but must contain the following property:
> +
>>>
>>> ...
>
> += APR DEVICES:
> +Each subnode of the APR node can represent devices tied to this apr,
> like
> +sound-card. The properties of these nodes are defined by the
> individual
> +bindings for the specific device.


 It's not a good design generally to mix different types of nodes at one
 level.
>>>
>>>
>>> I agree, may be I can split the services and devices into different
>>> subnodes
>>> like below, which should avoid mixing different types of nodes.
>>>
>>> Does this sound good to you?
>>
>>
>> Seems your original example wasn't so complete...
>>
> Yep, I will fix it in next version.
>>
>> I don't see why you need all these nodes in the first place. For a
>> single SoC, how much does all this vary?
>>
> It might not vary for a given SoC, but It does vary across the SoCs.
> Also the versions of each service are independent to each other.

Not sure I follow the last statement. Meaning firmware updates change
the services?

I don't see any versioning of services here.

>
>>>
>>> apr {
>>>  compatible = "qcom,apr-v2";
>>>  qcom,smd-channels = "apr_audio_svc";
>>>  qcom,apr-dest-domain-id = ;
>>>
>>>  apr-services {
>>>  q6core {
>>>  qcom,apr-svc-name = "CORE";
>>>  qcom,apr-svc-id = ;
>>>  compatible = "qcom,q6core";
>>>  };
>>>
>>>  q6afe: q6afe {
>>>  compatible = "qcom,q6afe";
>>>  qcom,apr-svc-name = "AFE";
>>>  qcom,apr-svc-id = ;
>>>  #sound-dai-cells = <1>;
>>>  };
>>>
>>>  q6asm: q6asm {
>>>  compatible = "qcom,q6asm";
>>>  qcom,apr-svc-name = "ASM";
>>>  qcom,apr-svc-id = ;
>>>  #sound-dai-cells = <1>;
>>>  };
>>>
>>>  q6adm: q6adm {
>>>  compatible = "qcom,q6adm";
>>>  qcom,apr-svc-name = "ADM";
>>>  qcom,apr-svc-id = ;
>>>  #sound-dai-cells = <0>;
>>>  };
>>
>>
>> All these DAI nodes could be a single node and the cell value be the
>> svc-id?
>
> No, DAI's here are both backends and frontends, and some of the 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Rob Herring]

On Tue, Feb 20, 2018 at 3:33 AM, Srinivas Kandagatla
 wrote:
> Thanks for your review comments,
>
>
> On 18/02/18 23:04, Rob Herring wrote:
>>
>> On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:
>>>
>>> Thanks for the review,
>>>
>>> On 13/02/18 23:12, Rob Herring wrote:

 On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org
 wrote:
>
> From: Srinivas Kandagatla 
>
> This patch add dt bindings for Qualcomm APR (Asynchronous Packet
> Router)
> bus driver. This bus is used for communicating with DSP which provides
> audio and various other services to cpu.
>
> Signed-off-by: Srinivas Kandagatla 
> ---
>.../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83
> ++
>1 file changed, 83 insertions(+)
>create mode 100644
> Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> new file mode 100644
> index ..1b95fbfed348
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> @@ -0,0 +1,83 @@
> +Qualcomm APR (Asynchronous Packet Router) binding
> +
> +This binding describes the Qualcomm APR. APR is a IPC protocol for
> +communication between Application processor and QDSP. APR is mainly
> +used for audio/voice services on the QDSP.
> +
> +- compatible:
> +   Usage: required
> +   Value type: 
> +   Definition: must be "qcom,apr-v", example
> "qcom,apr-v2"
> +
> +- qcom,apr-dest-domain-id
> +   Usage: required
> +   Value type: 
> +   Definition: Destination processor ID.
> +   Possible values are :
> +   1 - APR simulator
> +   2 - PC
> +   3 - MODEM
> +   4 - ADSP
> +   5 - APPS
> +   6 - MODEM2
> +   7 - APPS2
> +
> += APR SERVICES
> +Each subnode of the APR node can represent service tied to this apr.
> The name
> +of the nodes are not important. The properties of these nodes are
> defined
> +by the individual bindings for the specific service
> +- but must contain the following property:
> +
>>>
>>> ...
>
> += APR DEVICES:
> +Each subnode of the APR node can represent devices tied to this apr,
> like
> +sound-card. The properties of these nodes are defined by the
> individual
> +bindings for the specific device.


 It's not a good design generally to mix different types of nodes at one
 level.
>>>
>>>
>>> I agree, may be I can split the services and devices into different
>>> subnodes
>>> like below, which should avoid mixing different types of nodes.
>>>
>>> Does this sound good to you?
>>
>>
>> Seems your original example wasn't so complete...
>>
> Yep, I will fix it in next version.
>>
>> I don't see why you need all these nodes in the first place. For a
>> single SoC, how much does all this vary?
>>
> It might not vary for a given SoC, but It does vary across the SoCs.
> Also the versions of each service are independent to each other.

Not sure I follow the last statement. Meaning firmware updates change
the services?

I don't see any versioning of services here.

>
>>>
>>> apr {
>>>  compatible = "qcom,apr-v2";
>>>  qcom,smd-channels = "apr_audio_svc";
>>>  qcom,apr-dest-domain-id = ;
>>>
>>>  apr-services {
>>>  q6core {
>>>  qcom,apr-svc-name = "CORE";
>>>  qcom,apr-svc-id = ;
>>>  compatible = "qcom,q6core";
>>>  };
>>>
>>>  q6afe: q6afe {
>>>  compatible = "qcom,q6afe";
>>>  qcom,apr-svc-name = "AFE";
>>>  qcom,apr-svc-id = ;
>>>  #sound-dai-cells = <1>;
>>>  };
>>>
>>>  q6asm: q6asm {
>>>  compatible = "qcom,q6asm";
>>>  qcom,apr-svc-name = "ASM";
>>>  qcom,apr-svc-id = ;
>>>  #sound-dai-cells = <1>;
>>>  };
>>>
>>>  q6adm: q6adm {
>>>  compatible = "qcom,q6adm";
>>>  qcom,apr-svc-name = "ADM";
>>>  qcom,apr-svc-id = ;
>>>  #sound-dai-cells = <0>;
>>>  };
>>
>>
>> All these DAI nodes could be a single node and the cell value be the
>> svc-id?
>
> No, DAI's here are both backends and frontends, and some of the services
> like core, USM are not DAI's
>
> Are you also saying that we should have a single driver 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

Thanks for your review comments,

On 18/02/18 23:04, Rob Herring wrote:

On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:

Thanks for the review,

On 13/02/18 23:12, Rob Herring wrote:

On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org wrote:

From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
   .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 
++
   1 file changed, 83 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,apr-v", example "qcom,apr-v2"
+
+- qcom,apr-dest-domain-id
+   Usage: required
+   Value type: 
+   Definition: Destination processor ID.
+   Possible values are :
+   1 - APR simulator
+   2 - PC
+   3 - MODEM
+   4 - ADSP
+   5 - APPS
+   6 - MODEM2
+   7 - APPS2
+
+= APR SERVICES
+Each subnode of the APR node can represent service tied to this apr. The name
+of the nodes are not important. The properties of these nodes are defined
+by the individual bindings for the specific service
+- but must contain the following property:
+

...

+= APR DEVICES:
+Each subnode of the APR node can represent devices tied to this apr, like
+sound-card. The properties of these nodes are defined by the individual
+bindings for the specific device.


It's not a good design generally to mix different types of nodes at one
level.


I agree, may be I can split the services and devices into different subnodes
like below, which should avoid mixing different types of nodes.

Does this sound good to you?


Seems your original example wasn't so complete...


Yep, I will fix it in next version.

I don't see why you need all these nodes in the first place. For a
single SoC, how much does all this vary?


It might not vary for a given SoC, but It does vary across the SoCs.
Also the versions of each service are independent to each other.


apr {
 compatible = "qcom,apr-v2";
 qcom,smd-channels = "apr_audio_svc";
 qcom,apr-dest-domain-id = ;

 apr-services {
 q6core {
 qcom,apr-svc-name = "CORE";
 qcom,apr-svc-id = ;
 compatible = "qcom,q6core";
 };

 q6afe: q6afe {
 compatible = "qcom,q6afe";
 qcom,apr-svc-name = "AFE";
 qcom,apr-svc-id = ;
 #sound-dai-cells = <1>;
 };

 q6asm: q6asm {
 compatible = "qcom,q6asm";
 qcom,apr-svc-name = "ASM";
 qcom,apr-svc-id = ;
 #sound-dai-cells = <1>;
 };

 q6adm: q6adm {
 compatible = "qcom,q6adm";
 qcom,apr-svc-name = "ADM";
 qcom,apr-svc-id = ;
 #sound-dai-cells = <0>;
 };


All these DAI nodes could be a single node and the cell value be the
svc-id?
No, DAI's here are both backends and frontends, and some of the services 
like core, USM are not DAI's


Are you also saying that we should have a single driver entity for all 
these services?





 };

 apr-devices {
 audio {
 compatible = "qcom,msm8996-snd-card";


This is a pseudo device. Why not put it at the top level like other
sound cards?


APR bus depends on the state of DSP services, which can go off if the 
DSP crashes or DSP is stopped. If we remove this sound card out of apr 
bus then the sound card dependency on apr bus is totally lost.


Main purpose of having sound card under this bus is that the sound card 
should register/unregister depending up the apr channel presence/absence 
respectively.


thanks,
srini



 ...
 };
 };
};







+
+= EXAMPLE
+The following example represents a QDSP 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

Thanks for your review comments,

On 18/02/18 23:04, Rob Herring wrote:

On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:

Thanks for the review,

On 13/02/18 23:12, Rob Herring wrote:

On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org wrote:

From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
   .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 
++
   1 file changed, 83 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,apr-v", example "qcom,apr-v2"
+
+- qcom,apr-dest-domain-id
+   Usage: required
+   Value type: 
+   Definition: Destination processor ID.
+   Possible values are :
+   1 - APR simulator
+   2 - PC
+   3 - MODEM
+   4 - ADSP
+   5 - APPS
+   6 - MODEM2
+   7 - APPS2
+
+= APR SERVICES
+Each subnode of the APR node can represent service tied to this apr. The name
+of the nodes are not important. The properties of these nodes are defined
+by the individual bindings for the specific service
+- but must contain the following property:
+

...

+= APR DEVICES:
+Each subnode of the APR node can represent devices tied to this apr, like
+sound-card. The properties of these nodes are defined by the individual
+bindings for the specific device.


It's not a good design generally to mix different types of nodes at one
level.


I agree, may be I can split the services and devices into different subnodes
like below, which should avoid mixing different types of nodes.

Does this sound good to you?


Seems your original example wasn't so complete...


Yep, I will fix it in next version.

I don't see why you need all these nodes in the first place. For a
single SoC, how much does all this vary?


It might not vary for a given SoC, but It does vary across the SoCs.
Also the versions of each service are independent to each other.


apr {
 compatible = "qcom,apr-v2";
 qcom,smd-channels = "apr_audio_svc";
 qcom,apr-dest-domain-id = ;

 apr-services {
 q6core {
 qcom,apr-svc-name = "CORE";
 qcom,apr-svc-id = ;
 compatible = "qcom,q6core";
 };

 q6afe: q6afe {
 compatible = "qcom,q6afe";
 qcom,apr-svc-name = "AFE";
 qcom,apr-svc-id = ;
 #sound-dai-cells = <1>;
 };

 q6asm: q6asm {
 compatible = "qcom,q6asm";
 qcom,apr-svc-name = "ASM";
 qcom,apr-svc-id = ;
 #sound-dai-cells = <1>;
 };

 q6adm: q6adm {
 compatible = "qcom,q6adm";
 qcom,apr-svc-name = "ADM";
 qcom,apr-svc-id = ;
 #sound-dai-cells = <0>;
 };


All these DAI nodes could be a single node and the cell value be the
svc-id?
No, DAI's here are both backends and frontends, and some of the services 
like core, USM are not DAI's


Are you also saying that we should have a single driver entity for all 
these services?





 };

 apr-devices {
 audio {
 compatible = "qcom,msm8996-snd-card";


This is a pseudo device. Why not put it at the top level like other
sound cards?


APR bus depends on the state of DSP services, which can go off if the 
DSP crashes or DSP is stopped. If we remove this sound card out of apr 
bus then the sound card dependency on apr bus is totally lost.


Main purpose of having sound card under this bus is that the sound card 
should register/unregister depending up the apr channel presence/absence 
respectively.


thanks,
srini



 ...
 };
 };
};







+
+= EXAMPLE
+The following example represents a QDSP based sound card on a MSM8996 device
+which uses apr as 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Rob Herring]

On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:
> Thanks for the review,
> 
> On 13/02/18 23:12, Rob Herring wrote:
> > On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org 
> > wrote:
> > > From: Srinivas Kandagatla 
> > > 
> > > This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
> > > bus driver. This bus is used for communicating with DSP which provides
> > > audio and various other services to cpu.
> > > 
> > > Signed-off-by: Srinivas Kandagatla 
> > > ---
> > >   .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 
> > > ++
> > >   1 file changed, 83 insertions(+)
> > >   create mode 100644 
> > > Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
> > > b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> > > new file mode 100644
> > > index ..1b95fbfed348
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> > > @@ -0,0 +1,83 @@
> > > +Qualcomm APR (Asynchronous Packet Router) binding
> > > +
> > > +This binding describes the Qualcomm APR. APR is a IPC protocol for
> > > +communication between Application processor and QDSP. APR is mainly
> > > +used for audio/voice services on the QDSP.
> > > +
> > > +- compatible:
> > > + Usage: required
> > > + Value type: 
> > > + Definition: must be "qcom,apr-v", example "qcom,apr-v2"
> > > +
> > > +- qcom,apr-dest-domain-id
> > > + Usage: required
> > > + Value type: 
> > > + Definition: Destination processor ID.
> > > + Possible values are :
> > > + 1 - APR simulator
> > > + 2 - PC
> > > + 3 - MODEM
> > > + 4 - ADSP
> > > + 5 - APPS
> > > + 6 - MODEM2
> > > + 7 - APPS2
> > > +
> > > += APR SERVICES
> > > +Each subnode of the APR node can represent service tied to this apr. The 
> > > name
> > > +of the nodes are not important. The properties of these nodes are defined
> > > +by the individual bindings for the specific service
> > > +- but must contain the following property:
> > > +
> ...
> > > += APR DEVICES:
> > > +Each subnode of the APR node can represent devices tied to this apr, like
> > > +sound-card. The properties of these nodes are defined by the individual
> > > +bindings for the specific device.
> > 
> > It's not a good design generally to mix different types of nodes at one
> > level.
> 
> I agree, may be I can split the services and devices into different subnodes
> like below, which should avoid mixing different types of nodes.
> 
> Does this sound good to you?

Seems your original example wasn't so complete...

I don't see why you need all these nodes in the first place. For a 
single SoC, how much does all this vary?

> 
> apr {
> compatible = "qcom,apr-v2";
> qcom,smd-channels = "apr_audio_svc";
> qcom,apr-dest-domain-id = ;
> 
> apr-services {
> q6core {
> qcom,apr-svc-name = "CORE";
> qcom,apr-svc-id = ;
> compatible = "qcom,q6core";
> };
> 
> q6afe: q6afe {
> compatible = "qcom,q6afe";
> qcom,apr-svc-name = "AFE";
> qcom,apr-svc-id = ;
> #sound-dai-cells = <1>;
> };
> 
> q6asm: q6asm {
> compatible = "qcom,q6asm";
> qcom,apr-svc-name = "ASM";
> qcom,apr-svc-id = ;
> #sound-dai-cells = <1>;
> };
> 
> q6adm: q6adm {
> compatible = "qcom,q6adm";
> qcom,apr-svc-name = "ADM";
> qcom,apr-svc-id = ;
> #sound-dai-cells = <0>;
> };

All these DAI nodes could be a single node and the cell value be the 
svc-id?

> };
> 
> apr-devices {
> audio {
> compatible = "qcom,msm8996-snd-card";

This is a pseudo device. Why not put it at the top level like other 
sound cards?

> ...
> };
> };
> };
> 
> 
> 
> 
> > 
> > > +
> > > += EXAMPLE
> > > +The following example represents a QDSP based sound card on a MSM8996 
> > > device
> > > +which uses apr as communication between Apps and QDSP.
> > > +
> > > + apr {
> > > + compatible = "qcom,apr-v2";
> > > + qcom,smd-channels = "apr_audio_svc";
> > > + qcom,apr-dest-domain-id = ;
> > > +
> > > + q6core {
> > > + compatible = "qcom,q6core";
> > > + qcom,apr-svc-name = "CORE";
> > > + qcom,apr-svc-id = ;
> > > + };
> > 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Rob Herring]

On Wed, Feb 14, 2018 at 09:13:23AM +, Srinivas Kandagatla wrote:
> Thanks for the review,
> 
> On 13/02/18 23:12, Rob Herring wrote:
> > On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org 
> > wrote:
> > > From: Srinivas Kandagatla 
> > > 
> > > This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
> > > bus driver. This bus is used for communicating with DSP which provides
> > > audio and various other services to cpu.
> > > 
> > > Signed-off-by: Srinivas Kandagatla 
> > > ---
> > >   .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 
> > > ++
> > >   1 file changed, 83 insertions(+)
> > >   create mode 100644 
> > > Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
> > > b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> > > new file mode 100644
> > > index ..1b95fbfed348
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> > > @@ -0,0 +1,83 @@
> > > +Qualcomm APR (Asynchronous Packet Router) binding
> > > +
> > > +This binding describes the Qualcomm APR. APR is a IPC protocol for
> > > +communication between Application processor and QDSP. APR is mainly
> > > +used for audio/voice services on the QDSP.
> > > +
> > > +- compatible:
> > > + Usage: required
> > > + Value type: 
> > > + Definition: must be "qcom,apr-v", example "qcom,apr-v2"
> > > +
> > > +- qcom,apr-dest-domain-id
> > > + Usage: required
> > > + Value type: 
> > > + Definition: Destination processor ID.
> > > + Possible values are :
> > > + 1 - APR simulator
> > > + 2 - PC
> > > + 3 - MODEM
> > > + 4 - ADSP
> > > + 5 - APPS
> > > + 6 - MODEM2
> > > + 7 - APPS2
> > > +
> > > += APR SERVICES
> > > +Each subnode of the APR node can represent service tied to this apr. The 
> > > name
> > > +of the nodes are not important. The properties of these nodes are defined
> > > +by the individual bindings for the specific service
> > > +- but must contain the following property:
> > > +
> ...
> > > += APR DEVICES:
> > > +Each subnode of the APR node can represent devices tied to this apr, like
> > > +sound-card. The properties of these nodes are defined by the individual
> > > +bindings for the specific device.
> > 
> > It's not a good design generally to mix different types of nodes at one
> > level.
> 
> I agree, may be I can split the services and devices into different subnodes
> like below, which should avoid mixing different types of nodes.
> 
> Does this sound good to you?

Seems your original example wasn't so complete...

I don't see why you need all these nodes in the first place. For a 
single SoC, how much does all this vary?

> 
> apr {
> compatible = "qcom,apr-v2";
> qcom,smd-channels = "apr_audio_svc";
> qcom,apr-dest-domain-id = ;
> 
> apr-services {
> q6core {
> qcom,apr-svc-name = "CORE";
> qcom,apr-svc-id = ;
> compatible = "qcom,q6core";
> };
> 
> q6afe: q6afe {
> compatible = "qcom,q6afe";
> qcom,apr-svc-name = "AFE";
> qcom,apr-svc-id = ;
> #sound-dai-cells = <1>;
> };
> 
> q6asm: q6asm {
> compatible = "qcom,q6asm";
> qcom,apr-svc-name = "ASM";
> qcom,apr-svc-id = ;
> #sound-dai-cells = <1>;
> };
> 
> q6adm: q6adm {
> compatible = "qcom,q6adm";
> qcom,apr-svc-name = "ADM";
> qcom,apr-svc-id = ;
> #sound-dai-cells = <0>;
> };

All these DAI nodes could be a single node and the cell value be the 
svc-id?

> };
> 
> apr-devices {
> audio {
> compatible = "qcom,msm8996-snd-card";

This is a pseudo device. Why not put it at the top level like other 
sound cards?

> ...
> };
> };
> };
> 
> 
> 
> 
> > 
> > > +
> > > += EXAMPLE
> > > +The following example represents a QDSP based sound card on a MSM8996 
> > > device
> > > +which uses apr as communication between Apps and QDSP.
> > > +
> > > + apr {
> > > + compatible = "qcom,apr-v2";
> > > + qcom,smd-channels = "apr_audio_svc";
> > > + qcom,apr-dest-domain-id = ;
> > > +
> > > + q6core {
> > > + compatible = "qcom,q6core";
> > > + qcom,apr-svc-name = "CORE";
> > > + qcom,apr-svc-id = ;
> > > + };
> > > +
> > > + q6afe {
> > > + compatible = 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

Thanks for the review,

On 13/02/18 23:12, Rob Herring wrote:

On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org wrote:

From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
  .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 ++
  1 file changed, 83 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,apr-v", example "qcom,apr-v2"
+
+- qcom,apr-dest-domain-id
+   Usage: required
+   Value type: 
+   Definition: Destination processor ID.
+   Possible values are :
+   1 - APR simulator
+   2 - PC
+   3 - MODEM
+   4 - ADSP
+   5 - APPS
+   6 - MODEM2
+   7 - APPS2
+
+= APR SERVICES
+Each subnode of the APR node can represent service tied to this apr. The name
+of the nodes are not important. The properties of these nodes are defined
+by the individual bindings for the specific service
+- but must contain the following property:
+

...

+= APR DEVICES:
+Each subnode of the APR node can represent devices tied to this apr, like
+sound-card. The properties of these nodes are defined by the individual
+bindings for the specific device.


It's not a good design generally to mix different types of nodes at one
level.


I agree, may be I can split the services and devices into different 
subnodes like below, which should avoid mixing different types of nodes.


Does this sound good to you?

apr {
compatible = "qcom,apr-v2";
qcom,smd-channels = "apr_audio_svc";
qcom,apr-dest-domain-id = ;

apr-services {
q6core {
qcom,apr-svc-name = "CORE";
qcom,apr-svc-id = ;
compatible = "qcom,q6core";
};

q6afe: q6afe {
compatible = "qcom,q6afe";
qcom,apr-svc-name = "AFE";
qcom,apr-svc-id = ;
#sound-dai-cells = <1>;
};

q6asm: q6asm {
compatible = "qcom,q6asm";
qcom,apr-svc-name = "ASM";
qcom,apr-svc-id = ;
#sound-dai-cells = <1>;
};

q6adm: q6adm {
compatible = "qcom,q6adm";
qcom,apr-svc-name = "ADM";
qcom,apr-svc-id = ;
#sound-dai-cells = <0>;
};
};

apr-devices {
audio {
compatible = "qcom,msm8996-snd-card";
...
};
};
};







+
+= EXAMPLE
+The following example represents a QDSP based sound card on a MSM8996 device
+which uses apr as communication between Apps and QDSP.
+
+   apr {
+   compatible = "qcom,apr-v2";
+   qcom,smd-channels = "apr_audio_svc";
+   qcom,apr-dest-domain-id = ;
+
+   q6core {
+   compatible = "qcom,q6core";
+   qcom,apr-svc-name = "CORE";
+   qcom,apr-svc-id = ;
+   };
+
+   q6afe {
+   compatible = "qcom,q6afe";
+   qcom,apr-svc-name = "AFE";
+   qcom,apr-svc-id = ;
+   };
+
+   audio {
+   compatible = "qcom,msm8996-snd-card";
+   ...
+   };
+   };
--
2.15.1

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Srinivas Kandagatla]

Thanks for the review,

On 13/02/18 23:12, Rob Herring wrote:

On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org wrote:

From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
  .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 ++
  1 file changed, 83 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,apr-v", example "qcom,apr-v2"
+
+- qcom,apr-dest-domain-id
+   Usage: required
+   Value type: 
+   Definition: Destination processor ID.
+   Possible values are :
+   1 - APR simulator
+   2 - PC
+   3 - MODEM
+   4 - ADSP
+   5 - APPS
+   6 - MODEM2
+   7 - APPS2
+
+= APR SERVICES
+Each subnode of the APR node can represent service tied to this apr. The name
+of the nodes are not important. The properties of these nodes are defined
+by the individual bindings for the specific service
+- but must contain the following property:
+

...

+= APR DEVICES:
+Each subnode of the APR node can represent devices tied to this apr, like
+sound-card. The properties of these nodes are defined by the individual
+bindings for the specific device.


It's not a good design generally to mix different types of nodes at one
level.


I agree, may be I can split the services and devices into different 
subnodes like below, which should avoid mixing different types of nodes.


Does this sound good to you?

apr {
compatible = "qcom,apr-v2";
qcom,smd-channels = "apr_audio_svc";
qcom,apr-dest-domain-id = ;

apr-services {
q6core {
qcom,apr-svc-name = "CORE";
qcom,apr-svc-id = ;
compatible = "qcom,q6core";
};

q6afe: q6afe {
compatible = "qcom,q6afe";
qcom,apr-svc-name = "AFE";
qcom,apr-svc-id = ;
#sound-dai-cells = <1>;
};

q6asm: q6asm {
compatible = "qcom,q6asm";
qcom,apr-svc-name = "ASM";
qcom,apr-svc-id = ;
#sound-dai-cells = <1>;
};

q6adm: q6adm {
compatible = "qcom,q6adm";
qcom,apr-svc-name = "ADM";
qcom,apr-svc-id = ;
#sound-dai-cells = <0>;
};
};

apr-devices {
audio {
compatible = "qcom,msm8996-snd-card";
...
};
};
};







+
+= EXAMPLE
+The following example represents a QDSP based sound card on a MSM8996 device
+which uses apr as communication between Apps and QDSP.
+
+   apr {
+   compatible = "qcom,apr-v2";
+   qcom,smd-channels = "apr_audio_svc";
+   qcom,apr-dest-domain-id = ;
+
+   q6core {
+   compatible = "qcom,q6core";
+   qcom,apr-svc-name = "CORE";
+   qcom,apr-svc-id = ;
+   };
+
+   q6afe {
+   compatible = "qcom,q6afe";
+   qcom,apr-svc-name = "AFE";
+   qcom,apr-svc-id = ;
+   };
+
+   audio {
+   compatible = "qcom,msm8996-snd-card";
+   ...
+   };
+   };
--
2.15.1

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Rob Herring]

On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla 
> 
> This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
> bus driver. This bus is used for communicating with DSP which provides
> audio and various other services to cpu.
> 
> Signed-off-by: Srinivas Kandagatla 
> ---
>  .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 
> ++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
> b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> new file mode 100644
> index ..1b95fbfed348
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> @@ -0,0 +1,83 @@
> +Qualcomm APR (Asynchronous Packet Router) binding
> +
> +This binding describes the Qualcomm APR. APR is a IPC protocol for
> +communication between Application processor and QDSP. APR is mainly
> +used for audio/voice services on the QDSP.
> +
> +- compatible:
> + Usage: required
> + Value type: 
> + Definition: must be "qcom,apr-v", example "qcom,apr-v2"
> +
> +- qcom,apr-dest-domain-id
> + Usage: required
> + Value type: 
> + Definition: Destination processor ID.
> + Possible values are :
> + 1 - APR simulator
> + 2 - PC
> + 3 - MODEM
> + 4 - ADSP
> + 5 - APPS
> + 6 - MODEM2
> + 7 - APPS2
> +
> += APR SERVICES
> +Each subnode of the APR node can represent service tied to this apr. The name
> +of the nodes are not important. The properties of these nodes are defined
> +by the individual bindings for the specific service
> +- but must contain the following property:
> +
> +- qcom,apr-svc-id
> + Usage: required
> + Value type: 
> + Definition: APR Service ID, used for matching the service.
> + Possible values are :
> + 3 - DSP Core Service
> + 4 - Audio Front End Service.
> + 5 - Voice Stream Manager Service.
> + 6 - Voice processing manager.
> + 7 - Audio Stream Manager Service.
> + 8 - Audio Device Manager Service.
> + 9 - Multimode voice manager.
> + 10 - Core voice stream.
> + 11 - Core voice processor.
> + 12 - Ultrasound stream manager.
> + 13 - Listen stream manager.
> +
> +- qcom,apr-svc-name
> + Usage: required
> + Value type: 
> + Definition: User readable name of a APR service.
> +
> += APR DEVICES:
> +Each subnode of the APR node can represent devices tied to this apr, like
> +sound-card. The properties of these nodes are defined by the individual
> +bindings for the specific device.

It's not a good design generally to mix different types of nodes at one 
level.

> +
> += EXAMPLE
> +The following example represents a QDSP based sound card on a MSM8996 device
> +which uses apr as communication between Apps and QDSP.
> +
> + apr {
> + compatible = "qcom,apr-v2";
> + qcom,smd-channels = "apr_audio_svc";
> + qcom,apr-dest-domain-id = ;
> +
> + q6core {
> + compatible = "qcom,q6core";
> + qcom,apr-svc-name = "CORE";
> + qcom,apr-svc-id = ;
> + };
> +
> + q6afe {
> + compatible = "qcom,q6afe";
> + qcom,apr-svc-name = "AFE";
> + qcom,apr-svc-id = ;
> + };
> +
> + audio {
> + compatible = "qcom,msm8996-snd-card";
> + ...
> + };
> + };
> -- 
> 2.15.1
> 

Re: [PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[Rob Herring]

On Tue, Feb 13, 2018 at 04:58:13PM +, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla 
> 
> This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
> bus driver. This bus is used for communicating with DSP which provides
> audio and various other services to cpu.
> 
> Signed-off-by: Srinivas Kandagatla 
> ---
>  .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 
> ++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
> b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> new file mode 100644
> index ..1b95fbfed348
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> @@ -0,0 +1,83 @@
> +Qualcomm APR (Asynchronous Packet Router) binding
> +
> +This binding describes the Qualcomm APR. APR is a IPC protocol for
> +communication between Application processor and QDSP. APR is mainly
> +used for audio/voice services on the QDSP.
> +
> +- compatible:
> + Usage: required
> + Value type: 
> + Definition: must be "qcom,apr-v", example "qcom,apr-v2"
> +
> +- qcom,apr-dest-domain-id
> + Usage: required
> + Value type: 
> + Definition: Destination processor ID.
> + Possible values are :
> + 1 - APR simulator
> + 2 - PC
> + 3 - MODEM
> + 4 - ADSP
> + 5 - APPS
> + 6 - MODEM2
> + 7 - APPS2
> +
> += APR SERVICES
> +Each subnode of the APR node can represent service tied to this apr. The name
> +of the nodes are not important. The properties of these nodes are defined
> +by the individual bindings for the specific service
> +- but must contain the following property:
> +
> +- qcom,apr-svc-id
> + Usage: required
> + Value type: 
> + Definition: APR Service ID, used for matching the service.
> + Possible values are :
> + 3 - DSP Core Service
> + 4 - Audio Front End Service.
> + 5 - Voice Stream Manager Service.
> + 6 - Voice processing manager.
> + 7 - Audio Stream Manager Service.
> + 8 - Audio Device Manager Service.
> + 9 - Multimode voice manager.
> + 10 - Core voice stream.
> + 11 - Core voice processor.
> + 12 - Ultrasound stream manager.
> + 13 - Listen stream manager.
> +
> +- qcom,apr-svc-name
> + Usage: required
> + Value type: 
> + Definition: User readable name of a APR service.
> +
> += APR DEVICES:
> +Each subnode of the APR node can represent devices tied to this apr, like
> +sound-card. The properties of these nodes are defined by the individual
> +bindings for the specific device.

It's not a good design generally to mix different types of nodes at one 
level.

> +
> += EXAMPLE
> +The following example represents a QDSP based sound card on a MSM8996 device
> +which uses apr as communication between Apps and QDSP.
> +
> + apr {
> + compatible = "qcom,apr-v2";
> + qcom,smd-channels = "apr_audio_svc";
> + qcom,apr-dest-domain-id = ;
> +
> + q6core {
> + compatible = "qcom,q6core";
> + qcom,apr-svc-name = "CORE";
> + qcom,apr-svc-id = ;
> + };
> +
> + q6afe {
> + compatible = "qcom,q6afe";
> + qcom,apr-svc-name = "AFE";
> + qcom,apr-svc-id = ;
> + };
> +
> + audio {
> + compatible = "qcom,msm8996-snd-card";
> + ...
> + };
> + };
> -- 
> 2.15.1
> 

[PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[srinivas . kandagatla]

From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
 .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 ++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,apr-v", example "qcom,apr-v2"
+
+- qcom,apr-dest-domain-id
+   Usage: required
+   Value type: 
+   Definition: Destination processor ID.
+   Possible values are :
+   1 - APR simulator
+   2 - PC
+   3 - MODEM
+   4 - ADSP
+   5 - APPS
+   6 - MODEM2
+   7 - APPS2
+
+= APR SERVICES
+Each subnode of the APR node can represent service tied to this apr. The name
+of the nodes are not important. The properties of these nodes are defined
+by the individual bindings for the specific service
+- but must contain the following property:
+
+- qcom,apr-svc-id
+   Usage: required
+   Value type: 
+   Definition: APR Service ID, used for matching the service.
+   Possible values are :
+   3 - DSP Core Service
+   4 - Audio Front End Service.
+   5 - Voice Stream Manager Service.
+   6 - Voice processing manager.
+   7 - Audio Stream Manager Service.
+   8 - Audio Device Manager Service.
+   9 - Multimode voice manager.
+   10 - Core voice stream.
+   11 - Core voice processor.
+   12 - Ultrasound stream manager.
+   13 - Listen stream manager.
+
+- qcom,apr-svc-name
+   Usage: required
+   Value type: 
+   Definition: User readable name of a APR service.
+
+= APR DEVICES:
+Each subnode of the APR node can represent devices tied to this apr, like
+sound-card. The properties of these nodes are defined by the individual
+bindings for the specific device.
+
+= EXAMPLE
+The following example represents a QDSP based sound card on a MSM8996 device
+which uses apr as communication between Apps and QDSP.
+
+   apr {
+   compatible = "qcom,apr-v2";
+   qcom,smd-channels = "apr_audio_svc";
+   qcom,apr-dest-domain-id = ;
+
+   q6core {
+   compatible = "qcom,q6core";
+   qcom,apr-svc-name = "CORE";
+   qcom,apr-svc-id = ;
+   };
+
+   q6afe {
+   compatible = "qcom,q6afe";
+   qcom,apr-svc-name = "AFE";
+   qcom,apr-svc-id = ;
+   };
+
+   audio {
+   compatible = "qcom,msm8996-snd-card";
+   ...
+   };
+   };
-- 
2.15.1

[PATCH v3 01/25] dt-bindings: soc: qcom: Add bindings for APR bus

[srinivas . kandagatla]

From: Srinivas Kandagatla 

This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.

Signed-off-by: Srinivas Kandagatla 
---
 .../devicetree/bindings/soc/qcom/qcom,apr.txt  | 83 ++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..1b95fbfed348
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,83 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: must be "qcom,apr-v", example "qcom,apr-v2"
+
+- qcom,apr-dest-domain-id
+   Usage: required
+   Value type: 
+   Definition: Destination processor ID.
+   Possible values are :
+   1 - APR simulator
+   2 - PC
+   3 - MODEM
+   4 - ADSP
+   5 - APPS
+   6 - MODEM2
+   7 - APPS2
+
+= APR SERVICES
+Each subnode of the APR node can represent service tied to this apr. The name
+of the nodes are not important. The properties of these nodes are defined
+by the individual bindings for the specific service
+- but must contain the following property:
+
+- qcom,apr-svc-id
+   Usage: required
+   Value type: 
+   Definition: APR Service ID, used for matching the service.
+   Possible values are :
+   3 - DSP Core Service
+   4 - Audio Front End Service.
+   5 - Voice Stream Manager Service.
+   6 - Voice processing manager.
+   7 - Audio Stream Manager Service.
+   8 - Audio Device Manager Service.
+   9 - Multimode voice manager.
+   10 - Core voice stream.
+   11 - Core voice processor.
+   12 - Ultrasound stream manager.
+   13 - Listen stream manager.
+
+- qcom,apr-svc-name
+   Usage: required
+   Value type: 
+   Definition: User readable name of a APR service.
+
+= APR DEVICES:
+Each subnode of the APR node can represent devices tied to this apr, like
+sound-card. The properties of these nodes are defined by the individual
+bindings for the specific device.
+
+= EXAMPLE
+The following example represents a QDSP based sound card on a MSM8996 device
+which uses apr as communication between Apps and QDSP.
+
+   apr {
+   compatible = "qcom,apr-v2";
+   qcom,smd-channels = "apr_audio_svc";
+   qcom,apr-dest-domain-id = ;
+
+   q6core {
+   compatible = "qcom,q6core";
+   qcom,apr-svc-name = "CORE";
+   qcom,apr-svc-id = ;
+   };
+
+   q6afe {
+   compatible = "qcom,q6afe";
+   qcom,apr-svc-name = "AFE";
+   qcom,apr-svc-id = ;
+   };
+
+   audio {
+   compatible = "qcom,msm8996-snd-card";
+   ...
+   };
+   };
-- 
2.15.1