Re: [PATCH v4 01/20] soc: dt-bindings: qcom: apr: convert to yaml

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

Thanks Rob for the review,

On 13/08/2021 23:23, Rob Herring wrote:
> On Mon, Aug 09, 2021 at 12:23:20PM +0100, Srinivas Kandagatla wrote:
>> Convert APR bindings to yaml format so that we could add new bindings support.
>>
>> All the dsp services bindings are now part of apr bindings instead
>> of adding them to audio bindings.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   .../devicetree/bindings/soc/qcom/qcom,apr.txt | 134 ------------------
>>   .../bindings/soc/qcom/qcom,apr.yaml           | 127 +++++++++++++++++
>>   2 files changed, 127 insertions(+), 134 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
>>   create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
>> deleted file mode 100644
>> index 2e2f6dc351c0..000000000000
>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
>> +++ /dev/null
>> @@ -1,134 +0,0 @@

...

>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
>> new file mode 100644
>> index 000000000000..12650f7084f4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
>> @@ -0,0 +1,127 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/soc/qcom/qcom,apr.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Qualcomm APR (Asynchronous Packet Router) binding
>> +
>> +maintainers:
>> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> +
>> +description: |
>> +  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.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,apr
> 
> This wasn't a valid compatible before.

True, Will remove that in next version.
> 
>> +      - qcom,apr-v2
>> +
>> +  qcom,apr-domain:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [1, 2, 3, 4, 5, 6, 7]
>> +    description:
>> +      Selects the processor domain for apr
>> +        1 = APR simulator
>> +        2 = PC Domain
>> +        3 = Modem Domain
>> +        4 = ADSP Domain
>> +        5 = Application processor Domain
>> +        6 = Modem2 Domain
>> +        7 = Application Processor2 Domain
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +#APR Services
>> +patternProperties:
>> +  "^apr-service@[0-9a-e]$":
> 
> Based on reg, this should be: [3-9a-d]

Sure, will fix that.

> 
>> +    type: object
>> +    description:
>> +      APR node's client devices use subnodes for desired static port services.
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - qcom,q6core
>> +          - qcom,q6asm
>> +          - qcom,q6afe
>> +          - qcom,q6adm
>> +
>> +      reg:
>> +        enum: [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]
>> +        description:
>> +          APR Service ID
>> +            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,protection-domain:
>> +        $ref: /schemas/types.yaml#/definitions/string-array
>> +        description: protection domain service name and path for apr service
>> +          has dependency on.
>> +        items:
>> +          - const: avs/audio
>> +          - const: msm/adsp/audio_pd
> 
> You've dropped a bunch of possible values.
> 
PDR services used for APR driver are only these two audio services.
so the others are really list of other services that APR driver will not 
using.

I can retain them as it was before, but we can tidy this list up in a 
cleanup patch later.

--srini

>> +
>> +      '#address-cells':
>> +        const: 1
>> +
>> +      '#size-cells':
>> +        const: 0
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - qcom,apr-domain
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/soc/qcom,apr.h>
>> +    apr {
>> +        compatible = "qcom,apr-v2";
>> +        qcom,apr-domain = <APR_DOMAIN_ADSP>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        q6core: apr-service@3 {
>> +          compatible = "qcom,q6core";
>> +          reg = <APR_SVC_ADSP_CORE>;
>> +          qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd";
>> +        };
>> +
>> +        q6afe: apr-service@4 {
>> +          compatible = "qcom,q6afe";
>> +          reg = <APR_SVC_AFE>;
>> +          qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd";
>> +        };
>> +
>> +        q6asm: apr-service@7 {
>> +          compatible = "qcom,q6asm";
>> +          reg = <APR_SVC_ASM>;
>> +          qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd";
>> +        };
>> +
>> +        q6adm: apr-service@8 {
>> +          compatible = "qcom,q6adm";
>> +          reg = <APR_SVC_ADM>;
>> +          qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd";
>> +        };
>> +    };
>> -- 
>> 2.21.0
>>
>>