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@xxxxxxxxxxx>
> Tested-by: Mohammad Rafi Shaik <quic_mohs@xxxxxxxxxxx>
> ---
> .../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
