2023-09-26 23:13:09

by Stephan Gerhold

[permalink] [raw]
Subject: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi

Most MSM8916/MSM8939 devices use very similar setups for the modem,
because most of the device-specific details are abstracted by the modem
firmware. There are several definitions (status switches, DAI links
etc) that will be exactly the same for every board.

Introduce a common msm8916-modem-qdsp6.dtsi include that can be used to
simplify enabling the modem for such devices. By default the
digital/analog codec in the SoC/PMIC is used, but boards can define
additional codecs using the templates for Secondary and Quaternary
MI2S.

Signed-off-by: Stephan Gerhold <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi | 163 ++++++++++++++++++++++
1 file changed, 163 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
new file mode 100644
index 000000000000..ddd74d428406
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * msm8916-modem-qdsp6.dtsi describes the typical modem setup on MSM8916 devices
+ * (or similar SoCs) with audio routed via the QDSP6 services provided by the
+ * modem firmware. The digital/analog codec in the SoC/PMIC is used by default,
+ * but boards can define additional codecs using the templates for Secondary and
+ * Quaternary MI2S.
+ */
+
+#include <dt-bindings/sound/qcom,q6afe.h>
+#include <dt-bindings/sound/qcom,q6asm.h>
+
+&apr {
+ status = "okay";
+};
+
+&bam_dmux {
+ status = "okay";
+};
+
+&bam_dmux_dma {
+ status = "okay";
+};
+
+&lpass {
+ status = "reserved"; /* Controlled by QDSP6 */
+};
+
+&lpass_codec {
+ status = "okay";
+};
+
+&mba_mem {
+ status = "okay";
+};
+
+&mpss {
+ status = "okay";
+};
+
+&mpss_mem {
+ status = "okay";
+};
+
+&pm8916_codec {
+ status = "okay";
+};
+
+&q6afedai {
+ dai@16 {
+ reg = <PRIMARY_MI2S_RX>;
+ qcom,sd-lines = <0 1>;
+ };
+ dai@20 {
+ reg = <TERTIARY_MI2S_TX>;
+ qcom,sd-lines = <0 1>;
+ };
+};
+
+&q6asmdai {
+ dai@0 {
+ reg = <0>;
+ direction = <Q6ASM_DAI_RX>;
+ };
+ dai@1 {
+ reg = <1>;
+ direction = <Q6ASM_DAI_TX>;
+ };
+ dai@2 {
+ reg = <2>;
+ direction = <Q6ASM_DAI_RX>;
+ };
+ dai@3 {
+ reg = <3>;
+ direction = <Q6ASM_DAI_RX>;
+ is-compress-dai;
+ };
+};
+
+&sound {
+ compatible = "qcom,msm8916-qdsp6-sndcard";
+ model = "msm8916";
+
+ pinctrl-0 = <&cdc_pdm_default>;
+ pinctrl-1 = <&cdc_pdm_sleep>;
+ pinctrl-names = "default", "sleep";
+
+ multimedia1-dai-link {
+ link-name = "MultiMedia1";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
+ };
+ };
+
+ multimedia2-dai-link {
+ link-name = "MultiMedia2";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA2>;
+ };
+ };
+
+ multimedia3-dai-link {
+ link-name = "MultiMedia3";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA3>;
+ };
+ };
+
+ multimedia4-dai-link {
+ link-name = "MultiMedia4";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA4>;
+ };
+ };
+
+ sound_dai_primary: mi2s-primary-dai-link {
+ link-name = "Primary MI2S";
+ cpu {
+ sound-dai = <&q6afedai PRIMARY_MI2S_RX>;
+ };
+ platform {
+ sound-dai = <&q6routing>;
+ };
+ codec {
+ sound-dai = <&lpass_codec 0>, <&pm8916_codec 0>;
+ };
+ };
+
+ sound_dai_secondary: mi2s-secondary-dai-link {
+ link-name = "Secondary MI2S";
+ status = "disabled"; /* Needs extra codec configuration */
+ cpu {
+ sound-dai = <&q6afedai SECONDARY_MI2S_RX>;
+ };
+ platform {
+ sound-dai = <&q6routing>;
+ };
+ };
+
+ sound_dai_tertiary: mi2s-tertiary-dai-link {
+ link-name = "Tertiary MI2S";
+ cpu {
+ sound-dai = <&q6afedai TERTIARY_MI2S_TX>;
+ };
+ platform {
+ sound-dai = <&q6routing>;
+ };
+ codec {
+ sound-dai = <&lpass_codec 1>, <&pm8916_codec 1>;
+ };
+ };
+
+ sound_dai_quaternary: mi2s-quaternary-dai-link {
+ link-name = "Quaternary MI2S";
+ status = "disabled"; /* Needs extra codec configuration */
+ cpu {
+ sound-dai = <&q6afedai QUATERNARY_MI2S_RX>;
+ };
+ platform {
+ sound-dai = <&q6routing>;
+ };
+ };
+};

--
2.42.0


2023-09-27 02:22:49

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi

On Tue, Sep 26, 2023 at 08:49:24PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > Most MSM8916/MSM8939 devices use very similar setups for the modem,
> > because most of the device-specific details are abstracted by the modem
> > firmware. There are several definitions (status switches, DAI links
> > etc) that will be exactly the same for every board.
> >
> > Introduce a common msm8916-modem-qdsp6.dtsi include that can be used to
> > simplify enabling the modem for such devices. By default the
> > digital/analog codec in the SoC/PMIC is used, but boards can define
> > additional codecs using the templates for Secondary and Quaternary
> > MI2S.
> >
> > Signed-off-by: Stephan Gerhold <[email protected]>
> > ---
> I'd rather see at least one usage so that you aren't introducing
> effectively non-compiled code..
>

There are 10 usages in the rest of the patch series.
Is that enough? :D

IMHO it doesn't make sense to squash this with one of the device
patches, especially considering several of them are primarily authored
by others.

> > arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi | 163 ++++++++++++++++++++++
> > 1 file changed, 163 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> > new file mode 100644
> > index 000000000000..ddd74d428406
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> > @@ -0,0 +1,163 @@
> > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> > +/*
> > + * msm8916-modem-qdsp6.dtsi describes the typical modem setup on MSM8916 devices
> > + * (or similar SoCs) with audio routed via the QDSP6 services provided by the
> > + * modem firmware. The digital/analog codec in the SoC/PMIC is used by default,
> > + * but boards can define additional codecs using the templates for Secondary and
> > + * Quaternary MI2S.
> > + */
> > +
> > +#include <dt-bindings/sound/qcom,q6afe.h>
> > +#include <dt-bindings/sound/qcom,q6asm.h>
> > +
> > +&apr {
> > + status = "okay";
> > +};
> > +
> > +&bam_dmux {
> > + status = "okay";
> > +};
> > +
> > +&bam_dmux_dma {
> > + status = "okay";
> > +};
> > +
> > +&lpass {
> > + status = "reserved"; /* Controlled by QDSP6 */
> > +};
> > +
> > +&lpass_codec {
> > + status = "okay";
> > +};
> Any reason for it to stay disabled?
>

You mean in msm8916.dtsi? For the SoC dtsi we don't make assumptions
what devices use or not. There could be devices that ignore the internal
codec entirely. If there is nothing connected to the codec lpass_codec
should not be enabled (e.g. the msm8916-ufi.dtsi devices).

This include is a bit more "opinionated", to reduce duplication for
the most common setup. But it's separate and optional to use. The SoC
dtsi is included by everyone.

> > +
> > +&mba_mem {
> > + status = "okay";
> > +};
> > +
> > +&mpss {
> > + status = "okay";
> > +};
> > +
> > +&mpss_mem {
> > + status = "okay";
> > +};
> > +
> > +&pm8916_codec {
> > + status = "okay";
> > +};
> Ditto
>

Same as above.

> > + multimedia1-dai-link {
> > + link-name = "MultiMedia1";
> Newline before last property and subnodes, please
>

Thanks, will change this!
>
> > + sound_dai_secondary: mi2s-secondary-dai-link {
> > + link-name = "Secondary MI2S";
> > + status = "disabled"; /* Needs extra codec configuration */
> Hmm.. Potential good user of /omit-if-no-ref/?
>

AFAICT /omit-if-no-ref/ is for phandle references only. Basically it
would only work if you would somewhere reference the phandle:

list-of-sound-dais = <&sound_dai_primary &sound_dai_secondary>;

But this doesn't exist so /omit-if-no-ref/ cannot be used here.

Thanks,
Stephan

2023-09-27 02:58:17

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi

On 26.09.2023 18:51, Stephan Gerhold wrote:
> Most MSM8916/MSM8939 devices use very similar setups for the modem,
> because most of the device-specific details are abstracted by the modem
> firmware. There are several definitions (status switches, DAI links
> etc) that will be exactly the same for every board.
>
> Introduce a common msm8916-modem-qdsp6.dtsi include that can be used to
> simplify enabling the modem for such devices. By default the
> digital/analog codec in the SoC/PMIC is used, but boards can define
> additional codecs using the templates for Secondary and Quaternary
> MI2S.
>
> Signed-off-by: Stephan Gerhold <[email protected]>
> ---
I'd rather see at least one usage so that you aren't introducing
effectively non-compiled code..

> arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi | 163 ++++++++++++++++++++++
> 1 file changed, 163 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> new file mode 100644
> index 000000000000..ddd74d428406
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> @@ -0,0 +1,163 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +/*
> + * msm8916-modem-qdsp6.dtsi describes the typical modem setup on MSM8916 devices
> + * (or similar SoCs) with audio routed via the QDSP6 services provided by the
> + * modem firmware. The digital/analog codec in the SoC/PMIC is used by default,
> + * but boards can define additional codecs using the templates for Secondary and
> + * Quaternary MI2S.
> + */
> +
> +#include <dt-bindings/sound/qcom,q6afe.h>
> +#include <dt-bindings/sound/qcom,q6asm.h>
> +
> +&apr {
> + status = "okay";
> +};
> +
> +&bam_dmux {
> + status = "okay";
> +};
> +
> +&bam_dmux_dma {
> + status = "okay";
> +};
> +
> +&lpass {
> + status = "reserved"; /* Controlled by QDSP6 */
> +};
> +
> +&lpass_codec {
> + status = "okay";
> +};
Any reason for it to stay disabled?

> +
> +&mba_mem {
> + status = "okay";
> +};
> +
> +&mpss {
> + status = "okay";
> +};
> +
> +&mpss_mem {
> + status = "okay";
> +};
> +
> +&pm8916_codec {
> + status = "okay";
> +};
Ditto

[...]

> + multimedia1-dai-link {
> + link-name = "MultiMedia1";
Newline before last property and subnodes, please

[...]

> + sound_dai_secondary: mi2s-secondary-dai-link {
> + link-name = "Secondary MI2S";
> + status = "disabled"; /* Needs extra codec configuration */
Hmm.. Potential good user of /omit-if-no-ref/?

Konrad