[PATCH 0/2] Fix APR audio regression on 6.2-rc1
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(-)
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
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
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 servicepossible 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
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
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
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; }
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
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,
participants (2)
-
Bjorn Andersson -
Krzysztof Kozlowski -
Stephan Gerhold