[PATCH 0/2] Fix APR audio regression on 6.2-rc1

[PATCH 0/2] Fix APR audio regression on 6.2-rc1

[Stephan Gerhold]

These two patches fix regressions in the Qualcomm APR driver that 
prevent audio from working on MSM8916 (and likely MSM8996). In previous 
kernel releases the "qcom,protection-domain" property was optional, in 
6.2-rc1 it is now required. It should remain optional because the 
protection domain functionality is only supported starting with MSM8998
and is not present on older SoCs [1].

These patches should go as fixes into 6.2 to fix the regression.

[1]: https://lore.kernel.org/all/20200312120842.21991-1-sibis@codeaurora.org/

Stephan Gerhold (2):
  dt-bindings: soc: qcom: apr: Make qcom,protection-domain optional
    again
  soc: qcom: apr: Make qcom,protection-domain optional again

 .../devicetree/bindings/soc/qcom/qcom,apr-services.yaml      | 5 ++---
 drivers/soc/qcom/apr.c                                       | 3 ++-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.39.0

[PATCH 1/2] dt-bindings: soc: qcom: apr: Make qcom,protection-domain optional again

[Stephan Gerhold]

The protection domain functionality exists only in SoCs starting from
MSM8998 [1], while the APR bindings are also used on older platforms.

Commit 41288c305836 ("ASoC: dt-bindings: qcom,apr: Split services to
shared schema") made the "qcom,protection-domain" required but it
should remain optional to avoid dtbs_check warnings on older platforms,
e.g.:

arch/arm64/boot/dts/qcom/apq8096-db820c.dtb:
  apr: service@3: 'qcom,protection-domain' is a required property
  From schema: Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml

[1]: https://lore.kernel.org/all/20200312120842.21991-1-sibis@codeaurora.org/

Fixes: 41288c305836 ("ASoC: dt-bindings: qcom,apr: Split services to shared schema")
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../devicetree/bindings/soc/qcom/qcom,apr-services.yaml      | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
index 290555426c39..bdf482db32aa 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
@@ -39,8 +39,8 @@ properties:
   qcom,protection-domain:
     $ref: /schemas/types.yaml#/definitions/string-array
     description: |
-      Protection domain service name and path for APR service
-      possible values are::
+      Protection domain service name and path for APR service (if supported).
+      Possible values are::
       "avs/audio", "msm/adsp/audio_pd".
       "kernel/elf_loader", "msm/modem/wlan_pd".
       "tms/servreg", "msm/adsp/audio_pd".
@@ -49,6 +49,5 @@ properties:
 
 required:
   - reg
-  - qcom,protection-domain
 
 additionalProperties: true
-- 
2.39.0

Re: [PATCH 1/2] dt-bindings: soc: qcom: apr: Make qcom,protection-domain optional again

[Bjorn Andersson]

On Thu, Dec 29, 2022 at 04:16:47PM +0100, Stephan Gerhold wrote:
> The protection domain functionality exists only in SoCs starting from
> MSM8998 [1], while the APR bindings are also used on older platforms.
> 
> Commit 41288c305836 ("ASoC: dt-bindings: qcom,apr: Split services to
> shared schema") made the "qcom,protection-domain" required but it
> should remain optional to avoid dtbs_check warnings on older platforms,
> e.g.:
> 
> arch/arm64/boot/dts/qcom/apq8096-db820c.dtb:
>   apr: service@3: 'qcom,protection-domain' is a required property
>   From schema: Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
> 
> [1]: https://lore.kernel.org/all/20200312120842.21991-1-sibis@codeaurora.org/
> 
> Fixes: 41288c305836 ("ASoC: dt-bindings: qcom,apr: Split services to shared schema")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> ---
>  .../devicetree/bindings/soc/qcom/qcom,apr-services.yaml      | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
> index 290555426c39..bdf482db32aa 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr-services.yaml
> @@ -39,8 +39,8 @@ properties:
>    qcom,protection-domain:
>      $ref: /schemas/types.yaml#/definitions/string-array
>      description: |
> -      Protection domain service name and path for APR service
> -      possible values are::
> +      Protection domain service name and path for APR service (if supported).
> +      Possible values are::
>        "avs/audio", "msm/adsp/audio_pd".
>        "kernel/elf_loader", "msm/modem/wlan_pd".
>        "tms/servreg", "msm/adsp/audio_pd".
> @@ -49,6 +49,5 @@ properties:
>  
>  required:
>    - reg
> -  - qcom,protection-domain
>  
>  additionalProperties: true
> -- 
> 2.39.0
> 

Re: [PATCH 1/2] dt-bindings: soc: qcom: apr: Make qcom,protection-domain optional again

[Krzysztof Kozlowski]

On 29/12/2022 16:16, Stephan Gerhold wrote:
> The protection domain functionality exists only in SoCs starting from
> MSM8998 [1], while the APR bindings are also used on older platforms.
> 
> Commit 41288c305836 ("ASoC: dt-bindings: qcom,apr: Split services to
> shared schema") made the "qcom,protection-domain" required but it
> should remain optional to avoid dtbs_check warnings on older platforms,
> e.g.:
> 
> arch/arm64/boot/dts/qcom/apq8096-db820c.dtb:
>   apr: service@3: 'qcom,protection-domain' is a required property
>   From schema: Documentation/devicetree/bindings/soc/qcom/qcom,apr.yaml
> 
> [1]: https://lore.kernel.org/all/20200312120842.21991-1-sibis@codeaurora.org/
> 
> Fixes: 41288c305836 ("ASoC: dt-bindings: qcom,apr: Split services to shared schema")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 2/2] soc: qcom: apr: Make qcom,protection-domain optional again

[Stephan Gerhold]

APR should not fail if the service device tree node does not have
the qcom,protection-domain property, since this functionality does
not exist on older platforms such as MSM8916 and MSM8996.

Ignore -EINVAL (returned when the property does not exist) to fix
a regression on 6.2-rc1 that prevents audio from working:

  qcom,apr remoteproc0:smd-edge.apr_audio_svc.-1.-1:
    Failed to read second value of qcom,protection-domain
  qcom,apr remoteproc0:smd-edge.apr_audio_svc.-1.-1:
    Failed to add apr 3 svc

Fixes: 6d7860f5750d ("soc: qcom: apr: Add check for idr_alloc and of_property_read_string_index")
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/soc/qcom/apr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/apr.c b/drivers/soc/qcom/apr.c
index cd44f17dad3d..d51abb462ae5 100644
--- a/drivers/soc/qcom/apr.c
+++ b/drivers/soc/qcom/apr.c
@@ -461,9 +461,10 @@ static int apr_add_device(struct device *dev, struct device_node *np,
 		goto out;
 	}
 
+	/* Protection domain is optional, it does not exist on older platforms */
 	ret = of_property_read_string_index(np, "qcom,protection-domain",
 					    1, &adev->service_path);
-	if (ret < 0) {
+	if (ret < 0 && ret != -EINVAL) {
 		dev_err(dev, "Failed to read second value of qcom,protection-domain\n");
 		goto out;
 	}
-- 
2.39.0

Re: [PATCH 2/2] soc: qcom: apr: Make qcom,protection-domain optional again

[Bjorn Andersson]

On Thu, Dec 29, 2022 at 04:16:48PM +0100, Stephan Gerhold wrote:
> APR should not fail if the service device tree node does not have
> the qcom,protection-domain property, since this functionality does
> not exist on older platforms such as MSM8916 and MSM8996.
> 

Forgot that when I reviewed 6d7860f5750d, but you're right. Sorry about
that.

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> Ignore -EINVAL (returned when the property does not exist) to fix
> a regression on 6.2-rc1 that prevents audio from working:
> 
>   qcom,apr remoteproc0:smd-edge.apr_audio_svc.-1.-1:
>     Failed to read second value of qcom,protection-domain
>   qcom,apr remoteproc0:smd-edge.apr_audio_svc.-1.-1:
>     Failed to add apr 3 svc
> 
> Fixes: 6d7860f5750d ("soc: qcom: apr: Add check for idr_alloc and of_property_read_string_index")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  drivers/soc/qcom/apr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/apr.c b/drivers/soc/qcom/apr.c
> index cd44f17dad3d..d51abb462ae5 100644
> --- a/drivers/soc/qcom/apr.c
> +++ b/drivers/soc/qcom/apr.c
> @@ -461,9 +461,10 @@ static int apr_add_device(struct device *dev, struct device_node *np,
>  		goto out;
>  	}
>  
> +	/* Protection domain is optional, it does not exist on older platforms */
>  	ret = of_property_read_string_index(np, "qcom,protection-domain",
>  					    1, &adev->service_path);
> -	if (ret < 0) {
> +	if (ret < 0 && ret != -EINVAL) {
>  		dev_err(dev, "Failed to read second value of qcom,protection-domain\n");
>  		goto out;
>  	}
> -- 
> 2.39.0
> 

Re: [PATCH 0/2] Fix APR audio regression on 6.2-rc1

[Bjorn Andersson]

On Thu, 29 Dec 2022 16:16:46 +0100, Stephan Gerhold wrote:
> These two patches fix regressions in the Qualcomm APR driver that
> prevent audio from working on MSM8916 (and likely MSM8996). In previous
> kernel releases the "qcom,protection-domain" property was optional, in
> 6.2-rc1 it is now required. It should remain optional because the
> protection domain functionality is only supported starting with MSM8998
> and is not present on older SoCs [1].
> 
> [...]

Applied, thanks!

[1/2] dt-bindings: soc: qcom: apr: Make qcom,protection-domain optional again
      commit: 26658868354963afbff672ad6f7a85c44c311975
[2/2] soc: qcom: apr: Make qcom,protection-domain optional again
      commit: 599d41fb8ea8bd2a99ca9525dd69405020e43dda

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>