| Message ID | 1675700201-12890-4-git-send-email-quic_srivasam@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add SC7280 audioreach device tree nodes | expand |
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>; > + }; > + }; > + }; > + }; > + }; > +
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
On 09/02/2023 23:55, Stephen Boyd wrote: >> + >> + 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? No, this is correct. I fixed the glink-edge binding last year. > >> + 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. It is a clock driver which was not put into clk. Maybe because it is tightly tied to entire QDSP platform. Best regards, Krzysztof
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"; +}; 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"; + + 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"; + + required-opps = <&rpmhpd_opp_nom>; + + 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>; + + 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"; + #clock-cells = <2>; + }; + }; + }; + }; + }; + remoteproc_wpss: remoteproc@8a00000 { compatible = "qcom,sc7280-wpss-pil"; reg = <0 0x08a00000 0 0x10000>;