Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5207a28b-9c8c-5014-28c1-15635ad30143@linaro.org>
Date:   Fri, 11 Nov 2022 16:35:03 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH 02/10] ASoC: dt-bindings: qcom,apr: Split services to
 shared schema
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Banajit Goswami <bgoswami@quicinc.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>, alsa-devel@alsa-project.org,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Patrick Lai <plai@qti.qualcomm.com>,
        Srinivasa Rao Mandadapu <srivasam@qti.qualcomm.com>
References: <20221111113547.100442-1-krzysztof.kozlowski@linaro.org>
 <20221111113547.100442-3-krzysztof.kozlowski@linaro.org>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <20221111113547.100442-3-krzysztof.kozlowski@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 11/11/2022 11:35, Krzysztof Kozlowski wrote:
> The APR/GPR nodes are organized like:
> 
>    apr-or-gpr-device-node <- qcom,apr.yaml
>      apr-gpr-service@[0-9] <- qcom,apr.yaml
>        service-specific-components <- /schemas/sound/qcom,q6*.yaml
> 
> The schema for services (apr-gpr-service@[0-9]) already grows

I have not seen these grow or change alteast in the past 9 years.

Old APR (Elite f/w) and new GPR (AudioReach) interface provides access 
to static services on the DSP.

> considerably and is still quite not specific.  It allows several
> incorrect combinations, like adding a clock-controller to a APM device.

This should be fixed for sure for validation.

We had dedicated bindings per service before.

As the service has changed as part of new AudioReach Firmware, we could 
have added new bindings for these services again. But as we are dealing 
with the same audio hardware and clock resources a new bindings per 
service did not make sense. Since then we moved all the lpass audio 
ports and clocks related bindings to qcom,q6dsp-lpass-clocks.yaml and 
qcom,q6dsp-lpass-ports.yaml.

--srini

> Restricting it would complicate the schema even more.  Bringing new
> support for sound on Qualcomm SM8450 and SC8280XP SoC would grow it as
> well.
> 
> Simplify the qcom,apr.yaml by splitting the services to a shared file
> which will be:
> 1. Referenced by qcom,apr.yaml with additionalProperties:true,
> 2. Referenced by specific bindings for services with
>     additionalProperties:false (not yet in this commit).
> 
> While moving the code, add also required 'reg' and
> 'qcom,protection-domain' to further constrain the bindings.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   .../bindings/soc/qcom/qcom,apr-services.yaml  |  54 +++++++++
>   .../bindings/soc/qcom/qcom,apr.yaml           | 108 +-----------------
>   MAINTAINERS                                   |   2 +-
>   3 files changed, 58 insertions(+), 106 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
> new file mode 100644
> index 000000000000..290555426c39
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/qcom/qcom,apr-services.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm APR/GPR services shared parts
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> +
> +description:
> +  Common parts of a static service in Qualcomm APR/GPR (Asynchronous/Generic
> +  Packet Router).
> +
> +properties:
> +  reg:
> +    minimum: 1
> +    maximum: 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.
> +      GPR Service ID
> +        1 = Audio Process Manager Service
> +        2 = Proxy Resource Manager Service.
> +        3 = AMDB Service.
> +        4 = Voice processing manager.
> +
> +  qcom,protection-domain:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    description: |
> +      Protection domain service name and path for APR service
> +      possible values are::
> +      "avs/audio", "msm/adsp/audio_pd".
> +      "kernel/elf_loader", "msm/modem/wlan_pd".
> +      "tms/servreg", "msm/adsp/audio_pd".
> +      "tms/servreg", "msm/modem/wlan_pd".
> +      "tms/servreg", "msm/slpi/sensor_pd".
> +
> +required:
> +  - reg
> +  - qcom,protection-domain
> +
> +additionalProperties: true
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
> index 0a7a34cb2497..9302ffe567d6 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
> @@ -80,115 +80,13 @@ properties:
>     '#size-cells':
>       const: 0
>   
> -#APR/GPR Services
>   patternProperties:
>     "^service@[1-9a-d]$":
>       type: object
> +    $ref: /schemas/soc/qcom/qcom,apr-services.yaml
> +    additionalProperties: true
>       description:
> -      APR/GPR node's client devices use subnodes for desired static port services.
> -
> -    properties:
> -      compatible:
> -        enum:
> -          - qcom,q6core
> -          - qcom,q6asm
> -          - qcom,q6afe
> -          - qcom,q6adm
> -          - qcom,q6apm
> -          - qcom,q6prm
> -
> -      reg:
> -        minimum: 1
> -        maximum: 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.
> -          GPR Service ID
> -            1 = Audio Process Manager Service
> -            2 = Proxy Resource Manager Service.
> -            3 = AMDB Service.
> -            4 = Voice processing manager.
> -
> -      clock-controller:
> -        $ref: /schemas/sound/qcom,q6dsp-lpass-clocks.yaml#
> -        description: Qualcomm DSP LPASS clock controller
> -        unevaluatedProperties: false
> -
> -      dais:
> -        type: object
> -        oneOf:
> -          - $ref: /schemas/sound/qcom,q6apm-dai.yaml#
> -          - $ref: /schemas/sound/qcom,q6dsp-lpass-ports.yaml#
> -          - $ref: /schemas/sound/qcom,q6asm-dais.yaml#
> -        unevaluatedProperties: false
> -        description: Qualcomm DSP audio ports
> -
> -      routing:
> -        type: object
> -        $ref: /schemas/sound/qcom,q6adm-routing.yaml#
> -        unevaluatedProperties: false
> -        description: Qualcomm DSP LPASS audio routing
> -
> -      qcom,protection-domain:
> -        $ref: /schemas/types.yaml#/definitions/string-array
> -        description: protection domain service name and path for apr service
> -          possible values are
> -          "avs/audio", "msm/adsp/audio_pd".
> -          "kernel/elf_loader", "msm/modem/wlan_pd".
> -          "tms/servreg", "msm/adsp/audio_pd".
> -          "tms/servreg", "msm/modem/wlan_pd".
> -          "tms/servreg", "msm/slpi/sensor_pd".
> -
> -    allOf:
> -      - if:
> -          properties:
> -            compatible:
> -              enum:
> -                - qcom,q6afe
> -        then:
> -          properties:
> -            dais:
> -              properties:
> -                compatible:
> -                  const: qcom,q6afe-dais
> -
> -      - if:
> -          properties:
> -            compatible:
> -              enum:
> -                - qcom,q6apm
> -        then:
> -          properties:
> -            dais:
> -              properties:
> -                compatible:
> -                  enum:
> -                    - qcom,q6apm-dais
> -                    - qcom,q6apm-lpass-dais
> -
> -      - if:
> -          properties:
> -            compatible:
> -              enum:
> -                - qcom,q6asm
> -        then:
> -          properties:
> -            dais:
> -              properties:
> -                compatible:
> -                  const: qcom,q6asm-dais
> -
> -    additionalProperties: false
> +      APR/GPR static port services.
>   
>   required:
>     - compatible
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3df7b9a3f5ba..804e4a700477 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16893,7 +16893,7 @@ M:	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>   M:	Banajit Goswami <bgoswami@quicinc.com>
>   L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
>   S:	Supported
> -F:	Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
> +F:	Documentation/devicetree/bindings/soc/qcom/qcom,apr*
>   F:	Documentation/devicetree/bindings/sound/qcom,*
>   F:	drivers/soc/qcom/apr.c
>   F:	include/dt-bindings/sound/qcom,wcd9335.h