Skip to contentSkip to conversation

ADSP for SDM670 and sound on Google Pixel 3a

PATCH Superseded Richard Acayan 4 participants 26 messages Sep 22 – Oct 1

Subsystems (5)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

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]

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
remoteproc: qcom: pas: Add SDM670 ADSP support
arm64: dts: qcom: sdm670: add adsp
arm64: dts: qcom: sdm670-google: enable adsp
arm64: dts: qcom: sdm670-google: add sound card
arm64: dts: qcom: pm660l: add analog audio codec
arm64: dts: qcom: sdm670: add digital audio codec
arm64: dts: qcom: sdm670-google: configure analog and digital codecs

 .../mailbox/qcom,apcs-kpss-global.yaml        |   1 +
 .../bindings/remoteproc/qcom,adsp.yaml        |   1 +
 .../bindings/soc/qcom/qcom,aoss-qmp.yaml      |   1 +
 .../bindings/soc/qcom/qcom,smp2p.yaml         |   2 +-
 arch/arm64/boot/dts/qcom/pm660l.dtsi          |  40 ++++
 .../boot/dts/qcom/sdm670-google-common.dtsi   | 149 ++++++++++++++
 arch/arm64/boot/dts/qcom/sdm670.dtsi          | 190 ++++++++++++++++++
 drivers/remoteproc/qcom_q6v5_pas.c            |   1 +
 8 files changed, 384 insertions(+), 1 deletion(-)
Signature · 1 line
2.55.0
#1 lore raw

PATCH 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
index f91276822858..7862d68c9678 100644
@@ -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.55.0
Acked-by on v1 1/11
#2 lore raw

Re: [PATCH 01/11] dt-bindings: soc: qcom,smp2p: add sleepstate support

Quoted 7 lines · Richard Acayan

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.

Quoted 5 lines
---
 Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Best regards,
Krzysztof

#3 lore raw

PATCH 2/11 dt-bindings: mailbox: qcom: Add SDM670 APCS compatible

In reply to Richard Acayan

Add 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
index 39bc374300c3..74f24e07abc3 100644
@@ -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.55.0
#5 lore raw

PATCH 3/11 dt-bindings: soc: qcom: aoss: Add compatible for SDM670

In reply to Richard Acayan

Document 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
index d2ae2ae87a95..6672f441c20a 100644
@@ -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.55.0
#7 lore raw

PATCH 4/11 dt-bindings: remoteproc: qcom: adsp: Add SDM670 ADSP compatible

In reply to Richard Acayan

Add the compatible for the Audio DSP on SDM670.

---
 Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +1 -0
index 7e8ecae8e6cb..f0f6fe1f560b 100644
@@ -25,6 +25,7 @@ properties:
       - 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
Signature · 1 line
2.55.0
#8 lore raw

Re: [PATCH 04/11] dt-bindings: remoteproc: qcom: adsp: Add SDM670 ADSP compatible

Quoted 7 lines · Richard Acayan

Add the compatible for the Audio DSP on SDM670.

---
 Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml | 1 +
 1 file changed, 1 insertion(+)

You need to update constraints.

Best regards,
Krzysztof

#9 lore raw

PATCH 5/11 remoteproc: qcom: pas: Add SDM670 ADSP support

In reply to 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.

---
 drivers/remoteproc/qcom_q6v5_pas.c | 1 +
 1 file changed, 1 insertion(+)
drivers/remoteproc/qcom_q6v5_pas.c +1 -0
index ca8e61254c44..5272a462fc49 100644
@@ -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.55.0
#10 lore raw

Re: [PATCH 05/11] remoteproc: qcom: pas: Add SDM670 ADSP support

Quoted 14 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.

---
 drivers/remoteproc/qcom_q6v5_pas.c | 1 +
 1 file changed, 1 insertion(+)
drivers/remoteproc/qcom_q6v5_pas.c +0 -0
index ca8e61254c44..5272a462fc49 100644

drivers/remoteproc/qcom_q6v5_pas.c static const struct of_device_id qcom_pas_of_match[] = { · line 1868

Quoted 4 lines
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 },

Would a fallback compatible be suitable here?

Konrad

#11 lore raw

Re: [PATCH 05/11] remoteproc: qcom: pas: Add SDM670 ADSP support

Quoted 10 lines · Konrad Dybcio
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.

---
 drivers/remoteproc/qcom_q6v5_pas.c | 1 +
 1 file changed, 1 insertion(+)
Quoted 11 lines

drivers/remoteproc/qcom_q6v5_pas.c static const struct of_device_id qcom_pas_of_match[] = { · line 1868

Quoted 9 lines
drivers/remoteproc/qcom_q6v5_pas.c +1 -0
index ca8e61254c44..5272a462fc49 100644
@@ -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 },

Would a fallback compatible be suitable here?

I'll do this next revision to avoid adding the constraints in the bindings, but I think SDM845 will only have one specific compatible. MSM8998 and SM8150 use different data than SDM845.

#12 lore raw

PATCH 6/11 arm64: dts: qcom: sdm670: add adsp

In reply to 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_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 | 175 +++++++++++++++++++++++++++
 1 file changed, 175 insertions(+)
arch/arm64/boot/dts/qcom/sdm670.dtsi +175 -0
index 5c60ea36f04a..3856431fa587 100644
@@ -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,40 @@ 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>;
+		};
+
+		/*
+		 * The ADSP crashes if this is not present. Simply probing this
+		 * magically prevents the crash. It does not need any driver to
+		 * interact with it. It is labelled in case proper support is
+		 * added for it.
+		 */
+		adsp_smp2p_sleepstate: sleepstate {
+			qcom,entry-name = "sleepstate";
+			#qcom,smem-state-cells = <1>;
+		};
+	};
+
 	reserved-memory {
 		#address-cells = <2>;
 		#size-cells = <2>;
@@ -2111,6 +2146,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 +2239,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 +2355,127 @@ cpufreq_hw: cpufreq@17d43000 {
 			#freq-domain-cells = <1>;
 		};
 
+		adsp_pas: remoteproc@62400000 {
+			compatible = "qcom,sdm670-adsp-pas";
+			reg = <0 0x62400000 0 0x100>;
+
+			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>;
+						qcom,nsessions = <2>;
+					};
+				};
+			};
+		};
+
 		lpi_tlmm: pinctrl@62b40000 {
 			compatible = "qcom,sdm670-lpass-lpi-pinctrl";
 			reg = <0 0x62b40000 0 0x20000>;
Signature · 1 line
2.55.0
#13 lore raw

Re: [PATCH 06/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: sdm670: missing qcom,non-secure-domain in ADSP fastrpc --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: qcom: sdm670: missing qcom,non-secure-domain in ADSP fastrpc --

commit 9ab90abda949e62df6000bf0659ede91a3c191bb
Author: Richard Acayan <[email protected]>

arm64: dts: qcom: sdm670: add adsp

This commit adds the Audio DSP (ADSP) node and SMP2P nodes for SDM670 to handle audio and sensors. It includes the sleep state to prevent crashes and configures the GLINK edge with APR and FastRPC subnodes.

Quoted 3 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +0 -0

[ ... ]

Quoted 3 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +3 -0
+		adsp_pas: remoteproc@62400000 {
+			compatible = "qcom,sdm670-adsp-pas";
+			reg = <0 0x62400000 0 0x100>;

[ ... ]

Quoted 5 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +5 -0
+			glink-edge {
+				interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>;
+				label = "lpass";
+				qcom,remote-pid = <2>;
+				mboxes = <&apss_shared 24>;

[ ... ]

arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2449

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]
Does this fastrpc node need the qcom,non-secure-domain property?

Without this property, the secure_dsp flag evaluates to true during driver probe in drivers/misc/fastrpc.c. For the ADSP domain, fastrpc_device_register() will then create only the fastrpc-adsp-secure device node instead of fastrpc-adsp.

Will this cause standard userspace libraries to fail to open the DSP device if they expect the non-secure node?

Quoted 6 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +6 -0
+
+					compute-cb@3 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <3>;
+						iommus = <&apps_smmu 0x1803 0x0>;
+					};
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
#14 lore raw

Re: [PATCH 06/11] arm64: dts: qcom: sdm670: add adsp

In reply to Richard Acayan
Quoted 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_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 cluster_pd: power-domain-cluster { · line 535

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +10 -0
+		/*
+		 * The ADSP crashes if this is not present. Simply probing this
+		 * magically prevents the crash. It does not need any driver to
+		 * interact with it. It is labelled in case proper support is
+		 * added for it.
+		 */
+		adsp_smp2p_sleepstate: sleepstate {
+			qcom,entry-name = "sleepstate";
+			#qcom,smem-state-cells = <1>;
+		};

It's not magic, the driver registers it in the SMEM item, and the ADSP likely looks it up and complains if it's missing. You can drop this comment, since you already explained it in the commit message.

[...]

arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2360

Quoted 3 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +3 -0
+		adsp_pas: remoteproc@62400000 {
+			compatible = "qcom,sdm670-adsp-pas";
+			reg = <0 0x62400000 0 0x100>;

len=0x4040

[...] compute-cb@6 {

arch/arm64/boot/dts/qcom/sdm670.dtsi cpufreq_hw: cpufreq@17d43000 { · line 2473

Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm670.dtsi +4 -0
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <6>;
+						iommus = <&apps_smmu 0x1806 0x0>;
+						qcom,nsessions = <2>;

This property is going away, please remove it

Konrad

#15 lore raw

PATCH 7/11 arm64: dts: qcom: sdm670-google: enable adsp

In reply to Richard Acayan

Enable the ADSP on the Pixel 3a in anticipation of audio support.

---
 arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi | 5 +++++
 1 file changed, 5 insertions(+)
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +5 -0
index b4854801a5f5..70996b94cd33 100644
@@ -196,6 +196,11 @@ cam_vio_ldo: cam-vio-ldo-regulator {
 	};
 };
 
+&adsp_pas {
+	status = "okay";
+	firmware-name = "qcom/sdm670/sargo/adsp.mbn";
+};
+
 &apps_rsc {
 	regulators-0 {
 		compatible = "qcom,pm660-rpmh-regulators";
Signature · 1 line
2.55.0
#16 lore raw

Re: [PATCH 07/11] arm64: dts: qcom: sdm670-google: enable adsp

Quoted 12 lines · Richard Acayan

Enable the ADSP on the Pixel 3a in anticipation of audio support.

---
 arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi | 5 +++++
 1 file changed, 5 insertions(+)
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +0 -0
index b4854801a5f5..70996b94cd33 100644

arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi cam_vio_ldo: cam-vio-ldo-regulator { · line 202

Quoted 7 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +4 -0
@@ -196,6 +196,11 @@ cam_vio_ldo: cam-vio-ldo-regulator {
 	};
 };
 
+&adsp_pas {
+	status = "okay";
+	firmware-name = "qcom/sdm670/sargo/adsp.mbn";
+};

Please ensure 'status' is the last property with an \n before it

Konrad

#17 lore raw

PATCH 8/11 arm64: dts: qcom: sdm670-google: add sound card

In reply to Richard Acayan

The 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 and q6asm frontends, which can route simultaneously between the RT5514 microphone, CS35L36 stereo speakers, headset audio playback, and headset audio capture.

---
 .../boot/dts/qcom/sdm670-google-common.dtsi   | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +59 -0
index 70996b94cd33..3abde2cbf746 100644
@@ -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"
@@ -127,6 +128,46 @@ debug_info_mem: debug-info@a1800000 {
 		};
 	};
 
+	sound {
+		compatible = "qcom,sdm660-sndcard";
+		model = "Google Pixel 3a";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&cdc_pdm_default>;
+
+		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>;
+			};
+		};
+	};
+
 	/*
 	 * The touchscreen regulator seems to be controlled somehow by a gpio.
 	 * Model it as a fixed regulator and keep it on. Without schematics we
@@ -632,6 +673,24 @@ &qupv3_id_1 {
 	status = "okay";
 };
 
+&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.55.0
#18 lore raw

PATCH 9/11 arm64: dts: qcom: pm660l: add analog audio codec

In reply to 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.

---
 arch/arm64/boot/dts/qcom/pm660l.dtsi | 40 ++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
arch/arm64/boot/dts/qcom/pm660l.dtsi +40 -0
index 3f8b9eafe164..5b406b83043c 100644
@@ -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.55.0
#19 lore raw

Re: [PATCH 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: - [Low] dts: qcom: board properties hardcoded in generic pm660l.dtsi - [Low] dts: qcom: pm660l…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dts: qcom: board properties hardcoded in generic pm660l.dtsi
- [Low] dts: qcom: pm660l: sound-name-prefix violates binding schema
--

commit 331a31ea9c73f1abc1441342a8c3c461afc91cb0
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. Add the analog audio codec.

Quoted 4 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +0 -0
index 3f8b9eafe1641..5b406b83043c2 100644

[ ... ]

Quoted 8 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>;

[ ... ]

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;

[Severity: Low]
Are these properties appropriate for a generic PMIC device tree include?

These properties describe external, board-specific physical hardware rather than internal states of the PM660L PMIC. Including them in the shared .dtsi applies these board specifics to all devices using this PMIC, which forces boards with different headphone jack types or micbias wiring to use the Device Tree /delete-property/ anti-pattern to unset them.

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]
Does this property violate the binding schema?

The device tree schema for qcom,pm8916-wcd-analog-codec.yaml has
additionalProperties: false, but it does not define sound-name-prefix nor does it import a common schema (e.g. dai-common.yaml) that would permit it. This will likely trigger a dtbs_check schema validation error.

Quoted 4 lines
arch/arm64/boot/dts/qcom/pm660l.dtsi +4 -0
+			#sound-dai-cells = <1>;
+			status = "disabled";
+		};
+
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
#20 lore raw

PATCH 10/11 arm64: dts: qcom: sdm670: add digital audio codec

In reply to Richard Acayan

Add the SDM660 digital audio codec to the SDM670 device tree, as both platforms have the same internal sound card.

---
 arch/arm64/boot/dts/qcom/sdm670.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
arch/arm64/boot/dts/qcom/sdm670.dtsi +15 -0
index 3856431fa587..22a5ae0d01d5 100644
@@ -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>;
@@ -2548,5 +2549,19 @@ data2-pins {
 				};
 			};
 		};
+
+		lpass_codec: audio-codec@62ec0000 {
+			compatible = "qcom,sdm660-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.55.0
#21 lore raw

Re: [PATCH 10/11] arm64: dts: qcom: sdm670: add digital audio codec

Quoted 5 lines · Richard Acayan

Add the SDM660 digital audio codec to the SDM670 device tree, as both platforms have the same internal sound card.

---

Please introduce a SoC-specific compatible for 670.

Konrad

#22 lore raw

PATCH 11/11 arm64: dts: qcom: sdm670-google: configure analog and digital codecs

In reply to Richard Acayan

The SDM660 digital and PM660L analog audio codecs control the headphone jack on the Pixel 3a. Configure them for sound playback.

---
 .../boot/dts/qcom/sdm670-google-common.dtsi   | 87 ++++++++++++++++++-
 1 file changed, 86 insertions(+), 1 deletion(-)
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +86 -1
index 3abde2cbf746..48aea5d72092 100644
@@ -53,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;
@@ -133,7 +141,14 @@ sound {
 		model = "Google Pixel 3a";
 
 		pinctrl-names = "default";
-		pinctrl-0 = <&cdc_pdm_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";
@@ -166,6 +181,40 @@ cpu {
 				sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA4>;
 			};
 		};
+
+		lpi-mi2s-rx-dai-link {
+			link-name = "LPI MI2S Playback";
+
+			cpu {
+				sound-dai = <&q6afedai LPI_MI2S_RX_0>;
+			};
+
+			platform {
+				sound-dai = <&q6routing>;
+			};
+
+			codec {
+				sound-dai = <&pm660l_codec 0>,
+					    <&lpass_codec 0>;
+			};
+		};
+
+		lpi-mi2s-tx-dai-link {
+			link-name = "LPI MI2S Capture";
+
+			cpu {
+				sound-dai = <&q6afedai LPI_MI2S_TX_3>;
+			};
+
+			platform {
+				sound-dai = <&q6routing>;
+			};
+
+			codec {
+				sound-dai = <&pm660l_codec 1>,
+					    <&lpass_codec 1>;
+			};
+		};
 	};
 
 	/*
@@ -568,6 +617,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>;
@@ -631,6 +691,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";
 
@@ -673,6 +746,18 @@ &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>;
Signature · 1 line
2.55.0
#23 lore raw

Re: [PATCH 11/11] arm64: dts: qcom: sdm670-google: configure analog and digital codecs

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dts: qcom: audio bit clock disguised as AHB interconnect clock --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: qcom: audio bit clock disguised as AHB interconnect clock --

commit 31dd52373e653175568a580df559d7dd64860174
Author: Richard Acayan <[email protected]>

arm64: dts: qcom: sdm670-google: configure analog and digital codecs

The SDM660 digital and PM660L analog audio codecs control the headphone jack on the Pixel 3a. Configure them for sound playback.

Quoted 16 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +8 -0
index 3abde2cbf7462..48aea5d72092c 100644
@@ -53,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 623

Quoted 8 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +4 -0
@@ -568,6 +617,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 semantically correct to model an AHB bus clock as an audio bit clock?

The msm8916-wcd-digital codec driver requests an AHB interconnect clock ("ahbix-clk") for its register space. This patch maps that requirement to int_cdc_bclk, which is configured above as a 1.536MHz fixed clock (which corresponds exactly to a 48kHz audio BCLK).

While this allows the driver to probe successfully because a dummy fixed-clock returns 0 on clk_prepare_enable(), it misrepresents the hardware layout.

If the true AHB clock is abstracted, always-on, or managed elsewhere, shouldn't it be mapped to the board's XO or a generic system clock rather than a fake bit clock?

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
#24 lore raw

Re: [PATCH 11/11] arm64: dts: qcom: sdm670-google: configure analog and digital codecs

In reply to Richard Acayan
Quoted 5 lines · Richard Acayan

The SDM660 digital and PM660L analog audio codecs control the headphone jack on the Pixel 3a. Configure them for sound playback.

---

I think it's better to squash it with patch 10

[...]

Quoted 6 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +6 -0
+		lpi-mi2s-rx-dai-link {
+			link-name = "LPI MI2S Playback";
+
+			cpu {
+				sound-dai = <&q6afedai LPI_MI2S_RX_0>;
+			};

arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi cpu { · line 200

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +10 -0
+
+			platform {
+				sound-dai = <&q6routing>;
+			};
+
+			codec {
+				sound-dai = <&pm660l_codec 0>,
+					    <&lpass_codec 0>;
+			};
+		};

nit: 'co'dec , 'cp'u < 'p'latform

Konrad

#25 lore raw

Re: [PATCH 11/11] arm64: dts: qcom: sdm670-google: configure analog and digital codecs

Quoted 8 lines · Konrad Dybcio
Quoted 5 lines · Richard Acayan

The SDM660 digital and PM660L analog audio codecs control the headphone jack on the Pixel 3a. Configure them for sound playback.

---

I think it's better to squash it with patch 10

Maybe it would be better to combine with patch 8 (same /^.*:/ prefix), if this is just about number of patches. Patch 9 is about as related as patch 10.

Quoted 13 lines

[...]

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +10 -0
+		lpi-mi2s-rx-dai-link {
+			link-name = "LPI MI2S Playback";
+
+			cpu {
+				sound-dai = <&q6afedai LPI_MI2S_RX_0>;
+			};
+
+			platform {
+				sound-dai = <&q6routing>;
+			};
Quoted 8 lines

arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi cpu { · line 200

Quoted 6 lines
arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi +6 -0
+
+			codec {
+				sound-dai = <&pm660l_codec 0>,
+					    <&lpass_codec 0>;
+			};
+		};

nit: 'co'dec , 'cp'u < 'p'latform

(will be done in v2)