2022-12-22 10:00:41

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 3/7] arm64: dts: qcom: sc7280: audioreach: Add lpass pil node

Add lpass pil node for sc7280 based audioreach platforms.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Tested-by: Mohammad Rafi Shaik <[email protected]>
---
This patch depends on:
-- https://lore.kernel.org/linux-remoteproc/[email protected]/

.../qcom/sc7280-herobrine-audioreach-wcd9385.dtsi | 99 ++++++++++++++++++++++
1 file changed, 99 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 1eac94e1..0ce8755 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi
@@ -110,3 +110,102 @@
};
};
};
+
+&soc {
+ qcom,lpass@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 0>,
+ <&adsp_smp2p_in 1 0>,
+ <&adsp_smp2p_in 2 0>,
+ <&adsp_smp2p_in 3 0>,
+ <&adsp_smp2p_in 7 0>;
+
+ 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>,
+ <&lpasscc LPASS_QDSP6SS_XO_CLK>,
+ <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
+ <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
+
+ clock-names = "xo", "gcc_cfg_noc_lpass",
+ "lpass_qdsp6ss_xo",
+ "lpass_qdsp6ss_sleep",
+ "lpass_qdsp6ss_core";
+
+ iommus = <&apps_smmu 0x1800 0x0>;
+
+ power-domains = <&rpmhpd SC7280_LCX>;
+ power-domain-names = "lcx";
+ required-opps = <&rpmhpd_opp_nom>;
+
+ resets = <&aoss_reset AOSS_CC_LPASS_RESTART>,
+ <&pdc_reset PDC_AUDIO_SYNC_RESET>;
+
+ reset-names = "cc_lpass", "pdc_sync";
+
+ qcom,halt-regs = <&tcsr_1 0x3000 0x5000 0x8000 0x13000>;
+
+ memory-region = <&adsp_mem>;
+
+ status = "okay";
+
+ qcom,smem-states = <&adsp_smp2p_out 0>;
+ qcom,smem-state-names = "stop";
+
+ 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>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ qcom,intents = <512 20>;
+
+ q6apm: q6apm {
+ reg = <GPR_APM_MODULE_IID>;
+ compatible = "qcom,q6apm";
+ #sound-dai-cells = <0>;
+ q6apmdai: dais {
+ compatible = "qcom,q6apm-dais";
+ #sound-dai-cells = <1>;
+ iommus = <&apps_smmu 0x1801 0x0>;
+ };
+
+ q6apmbedai: bedais {
+ compatible = "qcom,q6apm-lpass-dais";
+ #sound-dai-cells = <1>;
+ };
+ };
+
+ q6prm: q6prm {
+ reg = <GPR_PRM_MODULE_IID>;
+ compatible = "qcom,q6prm";
+ #clock-cells = <2>;
+ q6prmcc: cc {
+ compatible = "qcom,q6prm-lpass-clocks";
+ #clock-cells = <2>;
+ };
+ };
+ };
+ };
+ };
+};
--
2.7.4


2022-12-23 09:55:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/7] arm64: dts: qcom: sc7280: audioreach: Add lpass pil node

On 22/12/2022 10:42, Srinivasa Rao Mandadapu wrote:
> Add lpass pil node for sc7280 based audioreach platforms.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Tested-by: Mohammad Rafi Shaik <[email protected]>
> ---
> This patch depends on:
> -- https://lore.kernel.org/linux-remoteproc/[email protected]/
>
> .../qcom/sc7280-herobrine-audioreach-wcd9385.dtsi | 99 ++++++++++++++++++++++
> 1 file changed, 99 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 1eac94e1..0ce8755 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi
> @@ -110,3 +110,102 @@
> };
> };
> };
> +
> +&soc {
> + qcom,lpass@3000000 {

Not correct name.

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Additionally, why this is not part of SoC DTSI? Defining SoC nodes in
other DTSI files is not readable and not maintainable.

> + compatible = "qcom,sc7280-adsp-pil";
> + reg = <0 0x03000000 0 0x5000>,
> + <0 0x0355b000 0 0x10>;

Misaligned.

> +
> + reg-names = "qdsp6ss_base",
> + "lpass_efuse";

Misaligned.

> +
> + interrupts-extended = <&pdc 6 IRQ_TYPE_LEVEL_HIGH>,
> + <&adsp_smp2p_in 0 0>,
> + <&adsp_smp2p_in 1 0>,
> + <&adsp_smp2p_in 2 0>,
> + <&adsp_smp2p_in 3 0>,
> + <&adsp_smp2p_in 7 0>;

Misaligned.

What are the trailing '0'?

> +
> + 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>,
> + <&lpasscc LPASS_QDSP6SS_XO_CLK>,
> + <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
> + <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
> +
> + clock-names = "xo", "gcc_cfg_noc_lpass",
> + "lpass_qdsp6ss_xo",
> + "lpass_qdsp6ss_sleep",
> + "lpass_qdsp6ss_core";
> +
> + iommus = <&apps_smmu 0x1800 0x0>;
> +
> + power-domains = <&rpmhpd SC7280_LCX>;
> + power-domain-names = "lcx";
> + required-opps = <&rpmhpd_opp_nom>;
> +
> + resets = <&aoss_reset AOSS_CC_LPASS_RESTART>,
> + <&pdc_reset PDC_AUDIO_SYNC_RESET>;
> +
> + reset-names = "cc_lpass", "pdc_sync";
> +
> + qcom,halt-regs = <&tcsr_1 0x3000 0x5000 0x8000 0x13000>;
> +
> + memory-region = <&adsp_mem>;
> +
> + status = "okay";

Why do you need this?
> +
> + qcom,smem-states = <&adsp_smp2p_out 0>;
> + qcom,smem-state-names = "stop";
> +
> + 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>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + qcom,intents = <512 20>;
> +
> + q6apm: q6apm {

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

It's not even passing regular test - build with W=1...

> + reg = <GPR_APM_MODULE_IID>;

Where did you include file that define?

> + compatible = "qcom,q6apm";
> + #sound-dai-cells = <0>;
> + q6apmdai: dais {
> + compatible = "qcom,q6apm-dais";
> + #sound-dai-cells = <1>;
> + iommus = <&apps_smmu 0x1801 0x0>;
> + };
> +

Best regards,
Krzysztof