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

Quoting Srinivasa Rao Mandadapu (2023-02-06 08:16:36)

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 {

•   status = "okay";
  

+};

Sort this file by phandle alphabetically?

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 @@ -3439,6 +3441,97 @@ status = "disabled"; };

•           remoteproc_adsp: remoteproc@3000000 {
  

This should be sorted on physical address. I think the node above is spi@88dc000 so this is in the wrong place.

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

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

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

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

•                                         <&adsp_smp2p_in 0 IRQ_TYPE_NONE>,
  

Can these have proper irq flags? Doubtful they're 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>;
  

Drop newline so clocks properties are together please.

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

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

•                   power-domains = <&rpmhpd SC7280_CX>;
  

•                   power-domain-names = "cx";
  

•                   required-opps = <&rpmhpd_opp_nom>;
  

•                   resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
  

•                            <&aoss_reset AOSS_CC_LPASS_RESTART>;
  

Drop newline so reset properties are together please.

•                   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 {
  

This node name should be apr per the qcom,glink-edge.yaml binding?

•                                   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>;
  

•                                           q6apmdai: dais {
  

•                                                   compatible = "qcom,q6apm-dais";
  

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

•                                           };
  

•                                           q6apmbedai: bedais {
  

•                                                   compatible = "qcom,q6apm-lpass-dais";
  

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

•                                           };
  

•                                   };
  

•                                   q6prm: service@2 {
  

•                                           compatible = "qcom,q6prm";
  

•                                           reg = <GPR_PRM_MODULE_IID>;
  

•                                           q6prmcc: clock-controller {
  

•                                                   compatible = "qcom,q6prm-lpass-clocks";
  

This is clk binding but not a clk driver? I'll look away now.

•                                                   #clock-cells = <2>;
  

•                                           };
  

•                                   };
  

•                           };
  

•                   };
  

•           };