[v5,3/8] arm64: dts: qcom: sc7280: Add LPASS PIL node
Commit Message
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(+)
Comments
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
@@ -107,3 +107,7 @@
};
};
};
+
+&remoteproc_adsp {
+ status = "okay";
+};
@@ -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>;