ADSP for SDM670 and sound on Google Pixel 3a
Subsystems (8)
v2 cover ADSP for SDM670 and sound on Google Pixel 3a
This series adds a user for the SDM660 sound support code [1]. It also adds the ADSP remoteproc for SDM670.
[1] https://lore.kernel.org/r/[email protected]
Changes since v1 (https://lore.kernel.org/r/[email protected]):
- squashed sound card and device-specific codec config (11/11)
- converted to fallback compatible (4/11)
- added compatible for WCD digital codec (5/11, 10/11)
- removed sleepstate comment (7/11)
- sorted subnodes of dai link (11/11)
- add review tags (1-3/11)
Richard Acayan (11):
dt-bindings: soc: qcom,smp2p: add sleepstate support
dt-bindings: mailbox: qcom: Add SDM670 APCS compatible
dt-bindings: soc: qcom: aoss: Add compatible for SDM670
dt-bindings: remoteproc: qcom: adsp: Add SDM670 ADSP compatible
ASoC: dt-bindings: msm8916-digital-codec: Add SDM670 compatible
remoteproc: qcom: pas: Add SDM670 ADSP support
arm64: dts: qcom: sdm670: add adsp
arm64: dts: qcom: sdm670-google: enable adsp
arm64: dts: qcom: pm660l: add analog audio codec
arm64: dts: qcom: sdm670: add digital audio codec
arm64: dts: qcom: sdm670-google: add sound card
.../mailbox/qcom,apcs-kpss-global.yaml | 1 + .../bindings/remoteproc/qcom,adsp.yaml | 33 ++-- .../bindings/soc/qcom/qcom,aoss-qmp.yaml | 1 + .../bindings/soc/qcom/qcom,smp2p.yaml | 2 +- .../sound/qcom,msm8916-wcd-digital-codec.yaml | 1 + arch/arm64/boot/dts/qcom/pm660l.dtsi | 40 ++++ .../boot/dts/qcom/sdm670-google-common.dtsi | 150 ++++++++++++++ arch/arm64/boot/dts/qcom/sdm670.dtsi | 183 ++++++++++++++++++ drivers/remoteproc/qcom_q6v5_pas.c | 1 + 9 files changed, 398 insertions(+), 14 deletions(-)
Signature · 1 line
2.56.0
PATCH v2 1/11 dt-bindings: soc: qcom,smp2p: add sleepstate support
The sleepstate entry is used by Qualcomm sensor DSPs to control suspend/resume operations. Even though there is no full support for sleep states, initialization of the entry is still necessary for some DSPs, notably the sensors-bearing ADSP on the Pixel 3a (SDM670 SoC). Allow sleepstates to be declared in the device tree so they can be initialized.
--- Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml +1 -1
@@ -60,7 +60,7 @@ properties: Two identifiers of the inbound and outbound smem items used for this edge. patternProperties: - "^master-kernel|slave-kernel|ipa-ap-to-modem|ipa-modem-to-ap|wlan-ap-to-wpss|wlan-wpss-to-ap$": + "^master-kernel|slave-kernel|ipa-ap-to-modem|ipa-modem-to-ap|sleepstate|wlan-ap-to-wpss|wlan-wpss-to-ap$": type: object description: Each SMP2P pair contain a set of inbound and outbound entries, these are
Signature · 1 line
2.56.0
PATCH v2 2/11 dt-bindings: mailbox: qcom: Add SDM670 APCS compatible
In reply to Richard AcayanAdd the compatible for the APCS in the SDM670 SoC.
--- .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml +1 -0
@@ -50,6 +50,7 @@ properties: - qcom,qcs615-apss-shared - qcom,sc7180-apss-shared - qcom,sc8180x-apss-shared + - qcom,sdm670-apss-shared - qcom,shikra-apss-shared - qcom,sm7150-apss-shared - qcom,sm8150-apss-shared
Signature · 1 line
2.56.0
Re: [PATCH v2 02/11] dt-bindings: mailbox: qcom: Add SDM670 APCS compatible
Quoted 22 lines · Richard Acayan
Add the compatible for the APCS in the SDM670 SoC.
Signed-off-by Richard Acayan <[email protected]>Reviewed-by Krzysztof Kozlowski <[email protected]>--- .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 1 + 1 file changed, 1 insertion(+)Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml +1
-1@@ -50,6 +50,7 @@ properties: - qcom,qcs615-apss-shared - qcom,sc7180-apss-shared - qcom,sc8180x-apss-shared + - qcom,sdm670-apss-shared - qcom,shikra-apss-shared - qcom,sm7150-apss-shared - qcom,sm8150-apss-shared --
Quoted 2 lines
2.56.0
Applied to mailbox/for-next
Thanks
Jassi
PATCH v2 3/11 dt-bindings: soc: qcom: aoss: Add compatible for SDM670
In reply to Richard AcayanDocument the compatible for SDM670.
--- Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.yaml | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.yaml +1 -0
@@ -43,6 +43,7 @@ properties: - qcom,sc8180x-aoss-qmp - qcom,sc8280xp-aoss-qmp - qcom,sdx75-aoss-qmp + - qcom,sdm670-aoss-qmp - qcom,sdm845-aoss-qmp - qcom,sm6350-aoss-qmp - qcom,sm7150-aoss-qmp
Signature · 1 line
2.56.0
PATCH v2 4/11 dt-bindings: remoteproc: qcom: adsp: Add SDM670 ADSP compatible
In reply to Richard AcayanThe SDM670 ADSP is like the SDM845 ADSP. Add the compatible for the Audio DSP on SDM670, with the fallback as SDM845.
--- .../bindings/remoteproc/qcom,adsp.yaml | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +20 -13
@@ -15,19 +15,26 @@ description: properties: compatible: - enum: - - qcom,msm8226-adsp-pil - - qcom,msm8953-adsp-pil - - qcom,msm8974-adsp-pil - - qcom,msm8996-adsp-pil - - qcom,msm8996-slpi-pil - - qcom,msm8998-adsp-pas - - qcom,msm8998-slpi-pas - - qcom,sdm660-adsp-pas - - qcom,sdm660-cdsp-pas - - qcom,sdm845-adsp-pas - - qcom,sdm845-cdsp-pas - - qcom,sdm845-slpi-pas + oneOf: + - enum: + - qcom,msm8226-adsp-pil + - qcom,msm8953-adsp-pil + - qcom,msm8974-adsp-pil + - qcom,msm8996-adsp-pil + - qcom,msm8996-slpi-pil + - qcom,msm8998-adsp-pas + - qcom,msm8998-slpi-pas + - qcom,sdm660-adsp-pas + - qcom,sdm660-cdsp-pas + - qcom,sdm670-adsp-pas + - qcom,sdm845-adsp-pas + - qcom,sdm845-cdsp-pas + - qcom,sdm845-slpi-pas + + - items: + - enum: + - qcom,sdm670-adsp-pas + - const: qcom,sdm845-adsp-pas reg: maxItems: 1
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v2 04/11] dt-bindings: remoteproc: qcom: adsp: Add SDM670 ADSP compatible
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dt-bindings: schema bypass due to compatible string in standalone enum --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: schema bypass due to compatible string in standalone enum --
commit fe965abba1a50ee3f23c3149e9b465705dc13459
Author: Richard Acayan <[email protected]>
dt-bindings: remoteproc: qcom: adsp: Add SDM670 ADSP compatible
The commit adds the SDM670 ADSP compatible string to the qcom,adsp binding. It splits the compatible property into a oneOf block to allow the SDM670 compatible to fallback to SDM845.
Quoted 8 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +0
-0@@ -15,19 +15,26 @@ description: properties: compatible:
[ ... ]
Quoted 3 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +3
-0+ oneOf: + - enum: + - qcom,msm8226-adsp-pil
[ ... ]
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +2
-0+ - qcom,sdm660-cdsp-pas + - qcom,sdm670-adsp-pas
[Severity: Medium]
Does adding qcom,sdm670-adsp-pas to this standalone enum branch bypass the fallback requirement?
Because oneOf only requires a single branch to match, a device tree node specifying only "qcom,sdm670-adsp-pas" matches this enum branch and passes schema validation.
This bypasses the fallback enforcement in the items branch below, allowing DTS nodes to be introduced without the required fallback string. Without the fallback string, older kernels may fail to probe the device.
Quoted 11 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +8
-0+ - qcom,sdm845-adsp-pas + - qcom,sdm845-cdsp-pas + - qcom,sdm845-slpi-pas + + - items: + - enum: + - qcom,sdm670-adsp-pas + - const: qcom,sdm845-adsp-pas reg: maxItems: 1
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Re: [PATCH v2 04/11] dt-bindings: remoteproc: qcom: adsp: Add SDM670 ADSP compatible
In reply to Richard AcayanQuoted 31 lines · Richard Acayan
The SDM670 ADSP is like the SDM845 ADSP. Add the compatible for the Audio DSP on SDM670, with the fallback as SDM845.
Signed-off-by Richard Acayan <[email protected]>--- .../bindings/remoteproc/qcom,adsp.yaml | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-)Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +2
-13@@ -15,19 +15,26 @@ description: properties: compatible: - enum: - - qcom,msm8226-adsp-pil - - qcom,msm8953-adsp-pil - - qcom,msm8974-adsp-pil - - qcom,msm8996-adsp-pil - - qcom,msm8996-slpi-pil - - qcom,msm8998-adsp-pas - - qcom,msm8998-slpi-pas - - qcom,sdm660-adsp-pas - - qcom,sdm660-cdsp-pas - - qcom,sdm845-adsp-pas - - qcom,sdm845-cdsp-pas - - qcom,sdm845-slpi-pas + oneOf: + - enum:
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml description: · line 29
Quoted 10 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +10
-0+ - qcom,msm8226-adsp-pil + - qcom,msm8953-adsp-pil + - qcom,msm8974-adsp-pil + - qcom,msm8996-adsp-pil + - qcom,msm8996-slpi-pil + - qcom,msm8998-adsp-pas + - qcom,msm8998-slpi-pas + - qcom,sdm660-adsp-pas + - qcom,sdm660-cdsp-pas + - qcom,sdm670-adsp-pas
Why is it here?
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml description: · line 36
Quoted 7 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +7
-0+ - qcom,sdm845-adsp-pas + - qcom,sdm845-cdsp-pas + - qcom,sdm845-slpi-pas + + - items: + - enum: + - qcom,sdm670-adsp-pas
And here?
How is it supposed to work?
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml description: · line 37
Quoted 1 line
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +1
-0+ - const: qcom,sdm845-adsp-pas
Best regards,
Krzysztof
PATCH v2 5/11 ASoC: dt-bindings: msm8916-digital-codec: Add SDM670 compatible
In reply to Richard AcayanThe MSM8916 digital codec is also found on SDM670, typically connected to the sound card. Add the compatible for SDM670.
--- .../bindings/sound/qcom,msm8916-wcd-digital-codec.yaml | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/sound/qcom,msm8916-wcd-digital-codec.yaml +1 -0
@@ -18,6 +18,7 @@ properties: - items: - enum: - qcom,sdm660-wcd-digital-codec + - qcom,sdm670-wcd-digital-codec - const: qcom,msm8916-wcd-digital-codec - const: qcom,msm8916-wcd-digital-codec
Signature · 1 line
2.56.0
Re: [PATCH v2 05/11] ASoC: dt-bindings: msm8916-digital-codec: Add SDM670 compatible
Quoted 7 lines · Richard Acayan
The MSM8916 digital codec is also found on SDM670, typically connected to the sound card. Add the compatible for SDM670.
Signed-off-by Richard Acayan <[email protected]>--- .../bindings/sound/qcom,msm8916-wcd-digital-codec.yaml | 1 + 1 file changed, 1 insertion(+)
Best regards,
Krzysztof
PATCH v2 6/11 remoteproc: qcom: pas: Add SDM670 ADSP support
In reply to Richard AcayanThe ADSP is used to control audio and sensors on the SDM670 SoC. Add the compatible for it, with the same data as SDM845 since the driver requires the load_state.
--- drivers/remoteproc/qcom_q6v5_pas.c | 1 + 1 file changed, 1 insertion(+)
drivers/remoteproc/qcom_q6v5_pas.c +1 -0
@@ -1865,6 +1865,7 @@ static const struct of_device_id qcom_pas_of_match[] = { { .compatible = "qcom,sc8280xp-nsp1-pas", .data = &sc8280xp_nsp1_resource }, { .compatible = "qcom,sdm660-adsp-pas", .data = &adsp_resource_init }, { .compatible = "qcom,sdm660-cdsp-pas", .data = &cdsp_resource_init }, + { .compatible = "qcom,sdm670-adsp-pas", .data = &sdm845_adsp_resource_init }, { .compatible = "qcom,sdm845-adsp-pas", .data = &sdm845_adsp_resource_init }, { .compatible = "qcom,sdm845-cdsp-pas", .data = &sdm845_cdsp_resource_init }, { .compatible = "qcom,sdm845-slpi-pas", .data = &sdm845_slpi_resource_init },
Signature · 1 line
2.56.0
Re: [PATCH v2 06/11] remoteproc: qcom: pas: Add SDM670 ADSP support
Quoted 9 lines · Richard Acayan
The ADSP is used to control audio and sensors on the SDM670 SoC. Add the compatible for it, with the same data as SDM845 since the driver requires the
load_state.Signed-off-by Richard Acayan <[email protected]>--- drivers/remoteproc/qcom_q6v5_pas.c | 1 + 1 file changed, 1 insertion(+)
Quoted 1 line
Signature · 2 lines
With best wishes Dmitry
PATCH v2 7/11 arm64: dts: qcom: sdm670: add adsp
In reply to Richard AcayanThe Audio DSP (Digital Signal Processor) is responsible for audio and sensors on SDM670. Add this, along with the SMP2P nodes used to communicate with it.
The sleep state is included to prevent the following crash:
qcom_q6v5_pas remoteproc-adsp: fatal error received: err_qdi.c:450:EF:sensor_process:1:sar.cc:27:chre_utils fatal
Otherwise, it is unused.
--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 168 +++++++++++++++++++++++++++ 1 file changed, 168 insertions(+)
arch/arm64/boot/dts/qcom/sdm670.dtsi +168 -0
@@ -19,6 +19,7 @@ #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/phy/phy-qcom-qusb2.h> #include <dt-bindings/power/qcom-rpmpd.h> +#include <dt-bindings/soc/qcom,apr.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> / {
@@ -500,6 +501,34 @@ cluster_pd: power-domain-cluster { }; }; + smp2p-lpass { + compatible = "qcom,smp2p"; + qcom,smem = <443>, <429>; + + interrupts = <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>; + + mboxes = <&apss_shared 26>; + + qcom,local-pid = <0>; + qcom,remote-pid = <2>; + + adsp_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + adsp_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + + adsp_smp2p_sleepstate: sleepstate { + qcom,entry-name = "sleepstate"; + #qcom,smem-state-cells = <1>; + }; + }; + reserved-memory { #address-cells = <2>; #size-cells = <2>;
@@ -2111,6 +2140,19 @@ dispcc: clock-controller@af00000 { #power-domain-cells = <1>; }; + aoss_qmp: power-management@c300000 { + compatible = "qcom,sdm670-aoss-qmp", "qcom,aoss-qmp"; + reg = <0 0x0c300000 0 0x100000>; + interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>; + mboxes = <&apss_shared 0>; + + #clock-cells = <0>; + + cx_cdev: cx { + #cooling-cells = <2>; + }; + }; + apps_smmu: iommu@15000000 { compatible = "qcom,sdm670-smmu-500", "qcom,smmu-500", "arm,mmu-500"; reg = <0 0x15000000 0 0x80000>;
@@ -2191,6 +2233,12 @@ gladiator_noc: interconnect@17900000 { qcom,bcm-voters = <&apps_bcm_voter>; }; + apss_shared: mailbox@17990000 { + compatible = "qcom,sdm670-apss-shared", "qcom,sdm845-apss-shared"; + reg = <0 0x17990000 0 0x1000>; + #mbox-cells = <1>; + }; + apps_rsc: rsc@179c0000 { compatible = "qcom,rpmh-rsc"; reg = <0 0x179c0000 0 0x10000>,
@@ -2301,6 +2349,126 @@ cpufreq_hw: cpufreq@17d43000 { #freq-domain-cells = <1>; }; + adsp_pas: remoteproc@62400000 { + compatible = "qcom,sdm670-adsp-pas"; + reg = <0 0x62400000 0 0x4040>; + + interrupts-extended = <&intc GIC_SPI 162 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "wdog", "fatal", "ready", + "handover", "stop-ack"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; + + memory-region = <&adsp_mem>; + + qcom,qmp = <&aoss_qmp>; + + qcom,smem-states = <&adsp_smp2p_out 0>; + qcom,smem-state-names = "stop"; + + status = "disabled"; + + glink-edge { + interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>; + label = "lpass"; + qcom,remote-pid = <2>; + mboxes = <&apss_shared 24>; + + apr { + compatible = "qcom,apr-v2"; + qcom,glink-channels = "apr_audio_svc"; + qcom,domain = <APR_DOMAIN_ADSP>; + #address-cells = <1>; + #size-cells = <0>; + qcom,intents = <512 20>; + + q6core: service@3 { + reg = <APR_SVC_ADSP_CORE>; + compatible = "qcom,q6core"; + qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd"; + }; + + q6afe: service@4 { + compatible = "qcom,q6afe"; + reg = <APR_SVC_AFE>; + qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd"; + + q6afecc: clock-controller { + compatible = "qcom,q6afe-clocks"; + #clock-cells = <2>; + }; + + q6afedai: dais { + compatible = "qcom,q6afe-dais"; + #address-cells = <1>; + #size-cells = <0>; + #sound-dai-cells = <1>; + }; + }; + + q6asm: service@7 { + compatible = "qcom,q6asm"; + reg = <APR_SVC_ASM>; + qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd"; + q6asmdai: dais { + compatible = "qcom,q6asm-dais"; + #address-cells = <1>; + #size-cells = <0>; + #sound-dai-cells = <1>; + iommus = <&apps_smmu 0x1801 0x0>; + }; + }; + + q6adm: service@8 { + compatible = "qcom,q6adm"; + reg = <APR_SVC_ADM>; + qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd"; + q6routing: routing { + compatible = "qcom,q6adm-routing"; + #sound-dai-cells = <0>; + }; + }; + }; + + fastrpc { + compatible = "qcom,fastrpc"; + qcom,glink-channels = "fastrpcglink-apps-dsp"; + label = "adsp"; + #address-cells = <1>; + #size-cells = <0>; + + compute-cb@3 { + compatible = "qcom,fastrpc-compute-cb"; + reg = <3>; + iommus = <&apps_smmu 0x1803 0x0>; + }; + + compute-cb@4 { + compatible = "qcom,fastrpc-compute-cb"; + reg = <4>; + iommus = <&apps_smmu 0x1804 0x0>; + }; + + compute-cb@5 { + compatible = "qcom,fastrpc-compute-cb"; + reg = <5>; + iommus = <&apps_smmu 0x1805 0x0>; + }; + + compute-cb@6 { + compatible = "qcom,fastrpc-compute-cb"; + reg = <6>; + iommus = <&apps_smmu 0x1806 0x0>; + }; + }; + }; + }; + lpi_tlmm: pinctrl@62b40000 { compatible = "qcom,sdm670-lpass-lpi-pinctrl"; reg = <0 0x62b40000 0 0x20000>;
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v2 07/11] arm64: dts: qcom: sdm670: add adsp
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dts: qcom: missing qcom,non-secure-domain in ADSP fastrpc node --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: qcom: missing qcom,non-secure-domain in ADSP fastrpc node --
commit 0239f40f83c2c0077ff48ea5393d0b2751dd608a
Author: Richard Acayan <[email protected]>
arm64: dts: qcom: sdm670: add adsp
This commit adds the Audio DSP (ADSP) node along with the SMP2P nodes used to communicate with it on the SDM670. The sleep state is also included to prevent crashes related to sensor processing.
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +0
-0
[ ... ]
Quoted 5 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +1
-0@@ -2301,6 +2349,126 @@ cpufreq_hw: cpufreq@17d43000 { #freq-domain-cells = <1>; }; + adsp_pas: remoteproc@62400000 {
[ ... ]
arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2443
Quoted 6 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +6
-0+ fastrpc { + compatible = "qcom,fastrpc"; + qcom,glink-channels = "fastrpcglink-apps-dsp"; + label = "adsp"; + #address-cells = <1>; + #size-cells = <0>;
[Severity: Medium]
Is the qcom,non-secure-domain property missing here?
During probe, the driver checks for qcom,non-secure-domain. When it is missing, the driver defaults secure_dsp to true, creating a /dev/fastrpc-adsp-secure device node instead of the standard /dev/fastrpc-adsp.
Does this break standard userspace audio and sensor applications that expect the non-secure device node for standard ADSP compute tasks?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
Re: [PATCH v2 07/11] arm64: dts: qcom: sdm670: add adsp
In reply to Richard AcayanQuoted 12 lines · Richard Acayan
The Audio DSP (Digital Signal Processor) is responsible for audio and sensors on SDM670. Add this, along with the SMP2P nodes used to communicate with it.
The sleep state is included to prevent the following crash:
qcom_q6v5_pasremoteproc-adsp: fatal error received: err_qdi.c:450:EF:sensor_process:1:sar.cc:27:chre_utils fatalOtherwise, it is unused.
Signed-off-by Richard Acayan <[email protected]>---
[...]
arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2417
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +4
-0+ q6asm: service@7 { + compatible = "qcom,q6asm"; + reg = <APR_SVC_ASM>; + qcom,protection-domain = "avs/audio", "msm/adsp/audio_pd";
1 a line would be neat
arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2418
Quoted 1 line
arch/arm64/boot/dts/qcom/sdm670.dtsi +1
-0+ q6asmdai: dais {
Please keep a \n between the last property and the following subnode, file-wide
otherwise looks good
Konrad
Re: [PATCH v2 07/11] arm64: dts: qcom: sdm670: add adsp
In reply to Richard AcayanQuoted 16 lines · Richard Acayan
The Audio DSP (Digital Signal Processor) is responsible for audio and sensors on SDM670. Add this, along with the SMP2P nodes used to communicate with it.
The sleep state is included to prevent the following crash:
qcom_q6v5_pasremoteproc-adsp: fatal error received: err_qdi.c:450:EF:sensor_process:1:sar.cc:27:chre_utils fatalOtherwise, it is unused.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 168 +++++++++++++++++++++++++++ 1 file changed, 168 insertions(+)
arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2353
Quoted 5 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +2
-0@@ -2301,6 +2349,126 @@ cpufreq_hw: cpufreq@17d43000 { #freq-domain-cells = <1>; }; + adsp_pas: remoteproc@62400000 { + compatible = "qcom,sdm670-adsp-pas";
Looking at your DT schema... Can we use "qcom,sdm845-adsp-pas" as a fallback here? Then you can drop a driver part.
arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2354
Quoted 1 line
arch/arm64/boot/dts/qcom/sdm670.dtsi +1
-0+ reg = <0 0x62400000 0 0x4040>;
0x0 instead of plain 0, please.
Signature · 2 lines
With best wishes Dmitry
Re: [PATCH v2 07/11] arm64: dts: qcom: sdm670: add adsp
Quoted 20 lines · Dmitry Baryshkov
Quoted 19 lines · Richard Acayan
The Audio DSP (Digital Signal Processor) is responsible for audio and sensors on SDM670. Add this, along with the SMP2P nodes used to communicate with it.
The sleep state is included to prevent the following crash:
qcom_q6v5_pasremoteproc-adsp: fatal error received: err_qdi.c:450:EF:sensor_process:1:sar.cc:27:chre_utils fatalOtherwise, it is unused.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 168 +++++++++++++++++++++++++++ 1 file changed, 168 insertions(+)arch/arm64/boot/dts/qcom/sdm670.dtsi +0
-0@@ -2301,6 +2349,126 @@ cpufreq_hw: cpufreq@17d43000 { #freq-domain-cells = <1>; };
Quoted 5 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2353
Quoted 2 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +2
-0+ adsp_pas: remoteproc@62400000 { + compatible = "qcom,sdm670-adsp-pas";Looking at your DT schema... Can we use "qcom,sdm845-adsp-pas" as a fallback here? Then you can drop a driver part.
Actually, looking at downstream, this should use the LCX power domain. Can power domains be omitted like on SDM845, or is LCX important enough?
Re: [PATCH v2 07/11] arm64: dts: qcom: sdm670: add adsp
Quoted 21 lines · Richard Acayan
Quoted 20 lines · Dmitry Baryshkov
Quoted 19 lines · Richard Acayan
The Audio DSP (Digital Signal Processor) is responsible for audio and sensors on SDM670. Add this, along with the SMP2P nodes used to communicate with it.
The sleep state is included to prevent the following crash:
qcom_q6v5_pasremoteproc-adsp: fatal error received: err_qdi.c:450:EF:sensor_process:1:sar.cc:27:chre_utils fatalOtherwise, it is unused.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 168 +++++++++++++++++++++++++++ 1 file changed, 168 insertions(+)arch/arm64/boot/dts/qcom/sdm670.dtsi +0
-0@@ -2301,6 +2349,126 @@ cpufreq_hw: cpufreq@17d43000 { #freq-domain-cells = <1>; };
Quoted 8 lines
Quoted 5 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2353
Quoted 2 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +2
-0+ adsp_pas: remoteproc@62400000 { + compatible = "qcom,sdm670-adsp-pas";Looking at your DT schema... Can we use "qcom,sdm845-adsp-pas" as a fallback here? Then you can drop a driver part.
Actually, looking at downstream, this should use the LCX power domain. Can power domains be omitted like on SDM845, or is LCX important enough?
It's used for "proxy voting" - i.e. the OS turns on the power so that the ADSP can boot and set up its own voting (through a direct interface to RPMH on its side), and then the OS drops its votes again
Konrad
PATCH v2 8/11 arm64: dts: qcom: sdm670-google: enable adsp
In reply to Richard AcayanEnable the ADSP on the Pixel 3a in anticipation of audio support.
--- arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +6 -0
@@ -196,6 +196,12 @@ cam_vio_ldo: cam-vio-ldo-regulator { }; }; +&adsp_pas { + firmware-name = "qcom/sdm670/sargo/adsp.mbn"; + + status = "okay"; +}; + &apps_rsc { regulators-0 { compatible = "qcom,pm660-rpmh-regulators";
Signature · 1 line
2.56.0
Re: [PATCH v2 08/11] arm64: dts: qcom: sdm670-google: enable adsp
Quoted 4 lines · Richard Acayan
Enable the ADSP on the Pixel 3a in anticipation of audio support.
Signed-off-by Richard Acayan <[email protected]>---
Konrad
Re: [PATCH v2 08/11] arm64: dts: qcom: sdm670-google: enable adsp
In reply to Richard AcayanQuoted 7 lines · Richard Acayan
Enable the ADSP on the Pixel 3a in anticipation of audio support.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi | 6 ++++++ 1 file changed, 6 insertions(+)
Signature · 2 lines
With best wishes Dmitry
PATCH v2 9/11 arm64: dts: qcom: pm660l: add analog audio codec
In reply to Richard AcayanThe PM660L analog audio codec (PMIC revision 4) controls the headphone jack, and optionally the earpiece and speaker, at the vendor's decision. Add the analog audio codec.
--- arch/arm64/boot/dts/qcom/pm660l.dtsi | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
arch/arm64/boot/dts/qcom/pm660l.dtsi +40 -0
@@ -87,6 +87,46 @@ pm660l_wled: leds@d800 { status = "disabled"; }; + pm660l_codec: audio-codec@f000 { + compatible = "qcom,pm660l-wcd-analog-codec", + "qcom,pm8953-wcd-analog-codec"; + reg = <0xf000>; + interrupts = <0x3 0xf0 0x0 IRQ_TYPE_NONE>, + <0x3 0xf0 0x1 IRQ_TYPE_NONE>, + <0x3 0xf0 0x2 IRQ_TYPE_NONE>, + <0x3 0xf0 0x3 IRQ_TYPE_NONE>, + <0x3 0xf0 0x4 IRQ_TYPE_NONE>, + <0x3 0xf0 0x5 IRQ_TYPE_NONE>, + <0x3 0xf0 0x6 IRQ_TYPE_NONE>, + <0x3 0xf0 0x7 IRQ_TYPE_NONE>, + <0x3 0xf1 0x0 IRQ_TYPE_NONE>, + <0x3 0xf1 0x1 IRQ_TYPE_NONE>, + <0x3 0xf1 0x2 IRQ_TYPE_NONE>, + <0x3 0xf1 0x3 IRQ_TYPE_NONE>, + <0x3 0xf1 0x4 IRQ_TYPE_NONE>, + <0x3 0xf1 0x5 IRQ_TYPE_NONE>; + interrupt-names = "cdc_spk_cnp_int", + "cdc_spk_clip_int", + "cdc_spk_ocp_int", + "mbhc_ins_rem_det1", + "mbhc_but_rel_det", + "mbhc_but_press_det", + "mbhc_ins_rem_det", + "mbhc_switch_int", + "cdc_ear_ocp_int", + "cdc_hphr_ocp_int", + "cdc_hphl_ocp_det", + "cdc_ear_cnp_int", + "cdc_hphr_cnp_int", + "cdc_hphl_cnp_int"; + qcom,hphl-jack-type-normally-open; + qcom,gnd-jack-type-normally-open; + qcom,micbias2-ext-cap; + sound-name-prefix = "Analog"; + #sound-dai-cells = <1>; + status = "disabled"; + }; + pm660l_spmi_regulators: regulators { compatible = "qcom,pm660l-regulators"; };
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v2 09/11] arm64: dts: qcom: pm660l: add analog audio codec
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] dts: pm660l: board-specific hardware properties hardcoded in generic PMIC dtsi - [Low] dts…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dts: pm660l: board-specific hardware properties hardcoded in generic PMIC dtsi
- [Low] dts: pm660l: undeclared sound-name-prefix causes dtbs_check warning --
commit cfcd4e3e10aad19a5a6d69fee31b064e8108d24a
Author: Richard Acayan <[email protected]>
arm64: dts: qcom: pm660l: add analog audio codec
The PM660L analog audio codec (PMIC revision 4) controls the headphone jack, and optionally the earpiece and speaker, at the vendor's decision. This commit adds the analog audio codec to the generic PMIC device tree.
Quoted 12 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +4
-0@@ -87,6 +87,46 @@ pm660l_wled: leds@d800 { status = "disabled"; }; + pm660l_codec: audio-codec@f000 { + compatible = "qcom,pm660l-wcd-analog-codec", + "qcom,pm8953-wcd-analog-codec"; + reg = <0xf000>;
[ ... ]
Quoted 7 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +7
-0+ interrupt-names = "cdc_spk_cnp_int", + "cdc_spk_clip_int", + "cdc_spk_ocp_int", + "mbhc_ins_rem_det1", + "mbhc_but_rel_det", + "mbhc_but_press_det", + "mbhc_ins_rem_det",
arch/arm64/boot/dts/qcom/pm660l.dtsi pm660l_wled: leds@d800 { · line 124
Quoted 10 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +10
-0+ "mbhc_switch_int", + "cdc_ear_ocp_int", + "cdc_hphr_ocp_int", + "cdc_hphl_ocp_det", + "cdc_ear_cnp_int", + "cdc_hphr_cnp_int", + "cdc_hphl_cnp_int"; + qcom,hphl-jack-type-normally-open; + qcom,gnd-jack-type-normally-open; + qcom,micbias2-ext-cap;
[Severity: Medium]
Will this cause problems for other boards that include this generic PMIC file but have different hardware wiring?
These physical wiring properties (qcom,hphl-jack-type-normally-open, qcom,gnd-jack-type-normally-open, qcom,micbias2-ext-cap) seem like they should be placed in board-specific .dts files rather than a generic common .dtsi.
Other boards with differently wired jacks (e.g., normally-closed) or lacking an external capacitor will inherit these settings, which could lead to inverted headphone detection or broken microphones.
arch/arm64/boot/dts/qcom/pm660l.dtsi pm660l_wled: leds@d800 { · line 125
Quoted 1 line
arch/arm64/boot/dts/qcom/pm660l.dtsi +1
-0+ sound-name-prefix = "Analog";
[Severity: Low]
Is this sound-name-prefix property supported by the dt-binding schema?
It appears that the dt-binding schema for this codec (Documentation/devicetree/bindings/sound/qcom,pm8916-wcd-analog-codec.yaml) enforces additionalProperties: false and does not define sound-name-prefix. Adding it here will likely cause a dtbs_check validation failure.
Quoted 7 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +4
-0+ #sound-dai-cells = <1>; + status = "disabled"; + }; + pm660l_spmi_regulators: regulators { compatible = "qcom,pm660l-regulators"; };
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
Re: [PATCH v2 09/11] arm64: dts: qcom: pm660l: add analog audio codec
In reply to Richard AcayanQuoted 14 lines · Richard Acayan
The PM660L analog audio codec (PMIC revision 4) controls the headphone jack, and optionally the earpiece and speaker, at the vendor's decision. Add the analog audio codec.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/pm660l.dtsi | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)arch/arm64/boot/dts/qcom/pm660l.dtsi +0
-0
arch/arm64/boot/dts/qcom/pm660l.dtsi pm660l_wled: leds@d800 { · line 95
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +6
-0@@ -87,6 +87,46 @@ pm660l_wled: leds@d800 { status = "disabled"; }; + pm660l_codec: audio-codec@f000 { + compatible = "qcom,pm660l-wcd-analog-codec", + "qcom,pm8953-wcd-analog-codec"; + reg = <0xf000>; + interrupts = <0x3 0xf0 0x0 IRQ_TYPE_NONE>, + <0x3 0xf0 0x1 IRQ_TYPE_NONE>,
I think all of them should be EDGE_BOTH
[...]
arch/arm64/boot/dts/qcom/pm660l.dtsi pm660l_wled: leds@d800 { · line 124
Quoted 3 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +3
-0+ qcom,hphl-jack-type-normally-open; + qcom,gnd-jack-type-normally-open; + qcom,micbias2-ext-cap;
And this is probably board-specific
Konrad
PATCH v2 10/11 arm64: dts: qcom: sdm670: add digital audio codec
In reply to Richard AcayanAdd the digital WCD audio codec to the SDM670 device tree.
--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
arch/arm64/boot/dts/qcom/sdm670.dtsi +15 -0
@@ -21,6 +21,7 @@ #include <dt-bindings/power/qcom-rpmpd.h> #include <dt-bindings/soc/qcom,apr.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> +#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h> / { interrupt-parent = <&intc>;
@@ -2541,5 +2542,19 @@ data2-pins { }; }; }; + + lpass_codec: audio-codec@62ec0000 { + compatible = "qcom,sdm670-wcd-digital-codec", + "qcom,msm8916-wcd-digital-codec"; + reg = <0 0x62ec0000 0 0x400>; + + clocks = <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + clock-names = "mclk"; + + sound-name-prefix = "Digital"; + #sound-dai-cells = <1>; + + status = "disabled"; + }; }; };
Signature · 1 line
2.56.0
Re: [PATCH v2 10/11] arm64: dts: qcom: sdm670: add digital audio codec
Quoted 21 lines · Richard Acayan
Add the digital WCD audio codec to the SDM670 device tree.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 15 +++++++++++++++ 1 file changed, 15 insertions(+)arch/arm64/boot/dts/qcom/sdm670.dtsi +1
-0@@ -21,6 +21,7 @@ #include <dt-bindings/power/qcom-rpmpd.h> #include <dt-bindings/soc/qcom,apr.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> +#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h> / { interrupt-parent = <&intc>;@@ -2541,5 +2542,19 @@ data2-pins { };
arch/arm64/boot/dts/qcom/sdm670.dtsi data2-pins { · line 2552
Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +8
-0}; }; + + lpass_codec: audio-codec@62ec0000 { + compatible = "qcom,sdm670-wcd-digital-codec", + "qcom,msm8916-wcd-digital-codec"; + reg = <0 0x62ec0000 0 0x400>; + + clocks = <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + clock-names = "mclk";
This clock configuration is forbidden according to bindings
I'm not expert in audio, but an LLM points me towards LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
Konrad
Re: [PATCH v2 10/11] arm64: dts: qcom: sdm670: add digital audio codec
Quoted 18 lines · Konrad Dybcio
Quoted 17 lines · Richard Acayan
Add the digital WCD audio codec to the SDM670 device tree.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 15 +++++++++++++++ 1 file changed, 15 insertions(+)arch/arm64/boot/dts/qcom/sdm670.dtsi +1
-0@@ -21,6 +21,7 @@ #include <dt-bindings/power/qcom-rpmpd.h> #include <dt-bindings/soc/qcom,apr.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> +#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
Quoted 16 lines
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +0
-0/ { interrupt-parent = <&intc>;@@ -2541,5 +2542,19 @@ data2-pins { };arch/arm64/boot/dts/qcom/sdm670.dtsi data2-pins { · line 2552
Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +8
-0}; }; + + lpass_codec: audio-codec@62ec0000 { + compatible = "qcom,sdm670-wcd-digital-codec", + "qcom,msm8916-wcd-digital-codec"; + reg = <0 0x62ec0000 0 0x400>; + + clocks = <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + clock-names = "mclk";This clock configuration is forbidden according to bindings
It is corrected in patch 11 which is why this isn't caught. I'll move the fixed clock to sdm670.dtsi.
Quoted 2 lines
I'm not expert in audio, but an LLM points me towards
LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
Does the LLM have access to schematics?
Re: [PATCH v2 10/11] arm64: dts: qcom: sdm670: add digital audio codec
Quoted 36 lines · Richard Acayan
Quoted 18 lines · Konrad Dybcio
Quoted 17 lines · Richard Acayan
Add the digital WCD audio codec to the SDM670 device tree.
Signed-off-by Richard Acayan <[email protected]>--- arch/arm64/boot/dts/qcom/sdm670.dtsi | 15 +++++++++++++++ 1 file changed, 15 insertions(+)arch/arm64/boot/dts/qcom/sdm670.dtsi +1
-0@@ -21,6 +21,7 @@ #include <dt-bindings/power/qcom-rpmpd.h> #include <dt-bindings/soc/qcom,apr.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> +#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>Quoted 16 lines
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +0
-0/ { interrupt-parent = <&intc>;@@ -2541,5 +2542,19 @@ data2-pins { };arch/arm64/boot/dts/qcom/sdm670.dtsi data2-pins { · line 2552
Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +8
-0}; }; + + lpass_codec: audio-codec@62ec0000 { + compatible = "qcom,sdm670-wcd-digital-codec", + "qcom,msm8916-wcd-digital-codec"; + reg = <0 0x62ec0000 0 0x400>; + + clocks = <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + clock-names = "mclk";This clock configuration is forbidden according to bindings
Quoted 7 lines
It is corrected in patch 11 which is why this isn't caught. I'll move the fixed clock to
sdm670.dtsi.Quoted 2 lines
I'm not expert in audio, but an LLM points me towards
LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_COREDoes the LLM have access to schematics?
That LLM has access to more than just schematics..
But at the end of the day it's just an LLM. I don't know for sure and that's why I wrote it that way.
Konrad
PATCH v2 11/11 arm64: dts: qcom: sdm670-google: add sound card
In reply to Richard AcayanThe Pixel 3a has a sound card for built-in audio (RT5514 and CS35L36 codecs) and for the headphone jack (WCD codec). Add the sound card, q6asm frontends, and WCD codec for headset audio.
--- .../boot/dts/qcom/sdm670-google-common.dtsi | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+)
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +144 -0
@@ -11,6 +11,7 @@ #include <dt-bindings/leds/common.h> #include <dt-bindings/pinctrl/qcom,pmic-gpio.h> #include <dt-bindings/power/qcom-rpmpd.h> +#include <dt-bindings/sound/qcom,q6asm.h> #include "sdm670.dtsi" #include "pm660.dtsi" #include "pm660l.dtsi"
@@ -52,6 +53,14 @@ framebuffer: framebuffer@9c000000 { }; }; + clocks { + int_cdc_bclk: int-cdc-bclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <1536000>; + }; + }; + gpio-keys { compatible = "gpio-keys"; autorepeat;
@@ -127,6 +136,87 @@ debug_info_mem: debug-info@a1800000 { }; }; + sound { + compatible = "qcom,sdm660-sndcard"; + model = "Google Pixel 3a"; + + pinctrl-names = "default"; + pinctrl-0 = <&cdc_pdm_default>, + <&cdc_dmic_default>; + + audio-routing = "Analog PDM_RX1", "Digital PDM_RX1", + "Analog PDM_RX2", "Digital PDM_RX2", + "Analog PDM_RX3", "Digital PDM_RX3", + "Digital LPASS_PDM_TX", "Analog PDM_TX", + "Analog AMIC2", "Analog MIC BIAS External2"; + + mm1-dai-link { + link-name = "MultiMedia1"; + + cpu { + sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>; + }; + }; + + mm2-dai-link { + link-name = "MultiMedia2"; + + cpu { + sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA2>; + }; + }; + + mm3-dai-link { + link-name = "MultiMedia3"; + + cpu { + sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA3>; + }; + }; + + mm4-dai-link { + link-name = "MultiMedia4"; + + cpu { + sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA4>; + }; + }; + + lpi-mi2s-rx-dai-link { + link-name = "LPI MI2S Playback"; + + codec { + sound-dai = <&pm660l_codec 0>, + <&lpass_codec 0>; + }; + + cpu { + sound-dai = <&q6afedai LPI_MI2S_RX_0>; + }; + + platform { + sound-dai = <&q6routing>; + }; + }; + + lpi-mi2s-tx-dai-link { + link-name = "LPI MI2S Capture"; + + codec { + sound-dai = <&pm660l_codec 1>, + <&lpass_codec 1>; + }; + + cpu { + sound-dai = <&q6afedai LPI_MI2S_TX_3>; + }; + + platform { + sound-dai = <&q6routing>; + }; + }; + }; + /* * The touchscreen regulator seems to be controlled somehow by a gpio. * Model it as a fixed regulator and keep it on. Without schematics we
@@ -528,6 +618,17 @@ rmi4_f12: rmi4-f12@12 { }; }; +&lpass_codec { + clocks = <&int_cdc_bclk>, + <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + clock-names = "ahbix-clk", "mclk"; + + assigned-clocks = <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + assigned-clock-rates = <9600000>; + + status = "okay"; +}; + &lpi_tlmm { /* sensor gpios are protected */ gpio-reserved-ranges = <0 8>, <12 6>;
@@ -591,6 +692,19 @@ &pm660_rradc { status = "okay"; }; +&pm660l_codec { + vdd-cdc-io-supply = <&vreg_l10a_1p8>; + vdd-cdc-tx-rx-cx-supply = <&vreg_l1b_0p925>; + vdd-micbias-supply = <&vreg_l7b_3p125>; + + qcom,mbhc-vthreshold-low = <75 150 237 437 437>; + qcom,mbhc-vthreshold-high = <75 150 237 437 437>; + + qcom,micbias-lvl = <2500>; + + status = "okay"; +}; + &pm660l_flash { status = "okay";
@@ -633,6 +747,36 @@ &qupv3_id_1 { status = "okay"; }; +&q6afedai { + dai@89 { + reg = <LPI_MI2S_RX_0>; + qcom,sd-lines = <0 1>; + }; + + dai@90 { + reg = <LPI_MI2S_TX_3>; + qcom,sd-lines = <0 1>; + }; +}; + +&q6asmdai { + dai@0 { + reg = <MSM_FRONTEND_DAI_MULTIMEDIA1>; + }; + + dai@1 { + reg = <MSM_FRONTEND_DAI_MULTIMEDIA2>; + }; + + dai@2 { + reg = <MSM_FRONTEND_DAI_MULTIMEDIA3>; + }; + + dai@3 { + reg = <MSM_FRONTEND_DAI_MULTIMEDIA4>; + }; +}; + &sdhc_1 { supports-cqe; mmc-hs200-1_8v;
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v2 11/11] arm64: dts: qcom: sdm670-google: add sound card
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dts: incorrect routing of 1.536MHz bit clock to AHB interconnect --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: incorrect routing of 1.536MHz bit clock to AHB interconnect --
commit dd0eb0dfbca83fed6d11405e093c6017f1613bfc
Author: Richard Acayan <[email protected]>
arm64: dts: qcom: sdm670-google: add sound card
This commit adds devicetree entries for the Pixel 3a sound card. It configures the WCD codec, q6asm frontends, audio routing, and the associated clocks for headset audio.
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +0
-0
[ ... ]
Quoted 12 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +8
-0@@ -52,6 +53,14 @@ framebuffer: framebuffer@9c000000 { }; }; + clocks { + int_cdc_bclk: int-cdc-bclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <1536000>; + }; + }; +
[ ... ]
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi rmi4_f12: rmi4-f12@12 { · line 624
Quoted 8 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +4
-0@@ -528,6 +618,17 @@ rmi4_f12: rmi4-f12@12 { }; }; +&lpass_codec { + clocks = <&int_cdc_bclk>, + <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + clock-names = "ahbix-clk", "mclk";
[Severity: Medium]
Is it safe to supply a dummy 1.536 MHz bit clock as the AHB interconnect clock here?
The underlying digital codec driver unconditionally requires the AHB clock in msm8916_wcd_digital_get_clks():
priv->ahbclk = devm_clk_get(dev, "ahbix-clk");
if (IS_ERR(priv->ahbclk)) {
dev_err(dev, "failed to get ahbix clk\n");
return PTR_ERR(priv->ahbclk);
}By satisfying "ahbix-clk" with a dummy fixed clock, the hardware state is misrepresented. The driver will successfully "enable" this dummy clock and proceed to access registers.
If the actual AHB clock is gated by firmware or another driver, could this result in register accesses without the bus being clocked, triggering a bus stall and a synchronous external abort?
Quoted 6 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +6
-0+ + assigned-clocks = <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + assigned-clock-rates = <9600000>; + + status = "okay"; +};
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
Mark Brown
maintainerRe: (subset) [PATCH v2 00/11] ADSP for SDM670 and sound on Google Pixel 3a
In reply to Richard AcayanQuoted 8 lines · Richard Acayan
ADSP for SDM670 and sound on Google Pixel 3a
This series adds a user for the SDM660 sound support code [1]. It also adds the ADSP remoteproc for SDM670.
[1] https://lore.kernel.org/r/[email protected]
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.4
Thanks!
[05/11] ASoC: dt-bindings: msm8916-digital-codec: Add SDM670 compatible
https://git.kernel.org/broonie/sound/c/5b81127873ae
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks,
Mark