Re: [PATCH v5 3/8] arm64: dts: qcom: sc7280: Add LPASS PIL node

On 06/02/2023 17:16, Srinivasa Rao Mandadapu wrote:

Add LPASS PIL node for sc7280 based audioreach platforms.

Signed-off-by: Srinivasa Rao Mandadapu quic_srivasam@quicinc.com Tested-by: Mohammad Rafi Shaik quic_mohs@quicinc.com

---

.../qcom/sc7280-herobrine-audioreach-wcd9385.dtsi | 4 + arch/arm64/boot/dts/qcom/sc7280.dtsi | 93 ++++++++++++++++++++++ 2 files changed, 97 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi index 1810a36..5e99f49 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi @@ -107,3 +107,7 @@ }; }; };

+&remoteproc_adsp {

Are you sure this is ordered by name?

• status = "okay";

+};

There is lpasscc@3000000 so aren't you now enabling two nodes for the same address?

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 6908bca..27ab992 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -8,6 +8,7 @@ #include <dt-bindings/clock/qcom,dispcc-sc7280.h> #include <dt-bindings/clock/qcom,gcc-sc7280.h> #include <dt-bindings/clock/qcom,gpucc-sc7280.h> +#include <dt-bindings/clock/qcom,lpass-sc7280.h> #include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h> #include <dt-bindings/clock/qcom,lpasscorecc-sc7280.h> #include <dt-bindings/clock/qcom,rpmh.h> @@ -21,6 +22,7 @@ #include <dt-bindings/power/qcom-rpmpd.h> #include <dt-bindings/reset/qcom,sdm845-aoss.h> #include <dt-bindings/reset/qcom,sdm845-pdc.h> +#include <dt-bindings/soc/qcom,gpr.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> #include <dt-bindings/sound/qcom,lpass.h> #include <dt-bindings/thermal/thermal.h> @@ -3439,6 +3441,97 @@ status = "disabled"; };

• remoteproc_adsp: remoteproc@3000000 {
  

• 	compatible = "qcom,sc7280-adsp-pil";
  

• 	reg = <0 0x03000000 0 0x5000>, <0 0x0355b000 0 0x10>;
  

• 	reg-names = "qdsp6ss_base", "lpass_efuse";
  

Does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions).

You can test against specific schema with DT_SCHEMA_FILES

• 	interrupts-extended = <&pdc 6 IRQ_TYPE_LEVEL_HIGH>,
  

• 			      <&adsp_smp2p_in 0 IRQ_TYPE_NONE>,
  

• 			      <&adsp_smp2p_in 1 IRQ_TYPE_NONE>,
  

• 			      <&adsp_smp2p_in 2 IRQ_TYPE_NONE>,
  

• 			      <&adsp_smp2p_in 3 IRQ_TYPE_NONE>,
  

• 			      <&adsp_smp2p_in 7 IRQ_TYPE_NONE>;
  

• 	interrupt-names = "wdog", "fatal", "ready",
  

• 			  "handover", "stop-ack",
  

• 			  "shutdown-ack";
  

• 	qcom,qmp = <&aoss_qmp>;
  

• 	clocks = <&rpmhcc RPMH_CXO_CLK>,
  

• 		 <&gcc GCC_CFG_NOC_LPASS_CLK>;
  

• 	clock-names = "xo", "gcc_cfg_noc_lpass";
  

• 	iommus = <&apps_smmu 0x1800 0x0>;
  

• 	power-domains =	<&rpmhpd SC7280_CX>;
  

• 	power-domain-names = "cx";
  

Here as well. Binding says it is LCX domain.

• 	required-opps = <&rpmhpd_opp_nom>;
  

This should also fail... please send code for review only if there are no warnings.

• 	resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
  

• 		 <&aoss_reset AOSS_CC_LPASS_RESTART>;
  

• 	reset-names =  "pdc_sync", "cc_lpass";
  

• 	qcom,halt-regs = <&tcsr_1 0x3000 0x5000 0x8000 0x13000>;
  

• 	memory-region = <&adsp_mem>;
  

• 	qcom,smem-states = <&adsp_smp2p_out 0>;
  

• 	qcom,smem-state-names = "stop";
  

• 	status = "disabled";
  

• 	glink-edge {
  

• 		interrupts-extended = <&ipcc IPCC_CLIENT_LPASS
  

• 				       IPCC_MPROC_SIGNAL_GLINK_QMP
  

• 				       IRQ_TYPE_EDGE_RISING>;
  

• 		mboxes = <&ipcc IPCC_CLIENT_LPASS
  

• 			 IPCC_MPROC_SIGNAL_GLINK_QMP>;
  

• 		label = "lpass";
  

• 		qcom,remote-pid = <2>;
  

• 		gpr {
  

• 			compatible = "qcom,gpr";
  

• 			qcom,glink-channels = "adsp_apps";
  

• 			qcom,domain = <GPR_DOMAIN_ID_ADSP>;
  

• 			qcom,intents = <512 20>;
  

• 			#address-cells = <1>;
  

• 			#size-cells = <0>;
  

• 			q6apm: service@1 {
  

• 				compatible = "qcom,q6apm";
  

• 				reg = <GPR_APM_MODULE_IID>;
  

• 				#sound-dai-cells = <0>;
  

That's also wrong and test would point the issue.

Best regards, Krzysztof