Re: [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs

From: Srinivasa Rao Mandadapu
Date: Thu Feb 03 2022 - 06:04:56 EST


On 1/6/2022 1:45 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
Quoting Srinivasa Rao Mandadapu (2022-01-03 03:12:58)
SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
codecs like WCD938x, max98360a using soundwire masters and i2s bus.
Add these nodes for sc7280 based platforms audio use case.
Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
---
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 4 ++
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 52 ++++++++++++++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 113 +++++++++++++++++++++++++++++++
3 files changed, 169 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index cd2755c..035cca9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -72,6 +72,10 @@ ap_ts_pen_1v8: &i2c13 {
pins = "gpio51";
};

+&wcd938x {
+ qcom,us-euro-gpios = <&tlmm 81 0>;
Why is this a qcom prefix vs. a standard foo-gpios property?
Okay. will remove qcom.

+};
+
&tlmm {
tp_int_odl: tp-int-odl {
pins = "gpio7";
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index ddeb508..94614c9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -636,3 +636,55 @@
bias-pull-up;
};
};
+
+&swr0 {
+ wcd_rx: wcd938x-rx{
space before { please

Also, are these speakers or amps or something like that? I'd expect the
node name to be more generic.
Okay. Will change accordingly.

+ compatible = "sdw20217010d00";
+ reg = <0 4>;
+ #sound-dai-cells = <1>;
+ qcom,rx-port-mapping = <1 2 3 4 5>;
+ };
+};
+
+&swr1 {
+ wcd_tx: wcd938x-tx{
+ compatible = "sdw20217010d00";
+ reg = <0 3>;
+ #sound-dai-cells = <1>;
+ qcom,tx-port-mapping = <1 2 3 4>;
+ };
+};
+
+&soc {
+ max98360a: audio-codec-0 {
This shouldn't be a child of the soc node.
Okay. Will move it to root node.

+ compatible = "maxim,max98360a";
+ pinctrl-names = "default";
+ pinctrl-0 = <&amp_en>;
+ sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
+ #sound-dai-cells = <0>;
+ };
+
+ wcd938x: codec {
Same comment.
Okay.

+ compatible = "qcom,wcd9380-codec";
+ #sound-dai-cells = <1>;
+
+ reset-gpios = <&tlmm 83 0>;
+
+ qcom,rx-device = <&wcd_rx>;
+ qcom,tx-device = <&wcd_tx>;
+
+ vdd-rxtx-supply = <&vreg_l18b_1p8>;
+ vdd-io-supply = <&vreg_l18b_1p8>;
+ vdd-buck-supply = <&vreg_l17b_1p8>;
+ vdd-mic-bias-supply = <&vreg_bob>;
+
+ qcom,micbias1-microvolt = <1800000>;
+ qcom,micbias2-microvolt = <1800000>;
+ qcom,micbias3-microvolt = <1800000>;
+ qcom,micbias4-microvolt = <1800000>;
+
+ qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
+ qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
+ qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 6233f2c..c0d9de3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1744,6 +1744,119 @@
#clock-cells = <1>;
};

+ rxmacro: rxmacro@3200000 {
What is rxmacro? Maybe 'soundwire'?
No. it's not soundwire. it's digital codec macro internal to LPASS.

+ pinctrl-names = "default";
+ pinctrl-0 = <&rx_swr_active>;
+ compatible = "qcom,sc7280-lpass-rx-macro";
+ reg = <0 0x3200000 0 0x1000>;
+
+ clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+ <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+ <&vamacro>;
+ clock-names = "mclk", "npl", "fsgen";
+ #clock-cells = <0>;
+ clock-frequency = <9600000>;
+ clock-output-names = "mclk";
+ #sound-dai-cells = <1>;
+ };
+
+ /* rx macro */
+ swr0: soundwire-controller@3210000 {
Is 'controller' redundant? i.e. soundwire is always a controller? Maybe
'soundwire' is better
Okay. will remove controller.

+ reg = <0 0x3210000 0 0x2000>;
+ compatible = "qcom,soundwire-v1.6.0";
+ interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rxmacro>;
+ clock-names = "iface";
+ label = "RX";
+
+ qcom,din-ports = <0>;
+ qcom,dout-ports = <5>;
+ qcom,swrm-hctl-reg = <0x032a90a0>;
+
+ qcom,ports-word-length = /bits/ 8 <0x01 0x07 0x04 0xFF 0xFF>;
+ qcom,ports-sinterval-low = /bits/ 8 <0x03 0x3F 0x1F 0x03 0x03>;
+ qcom,ports-offset1 = /bits/ 8 <0x00 0x00 0x0B 0x01 0x01>;
+ qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x0B 0x00 0x00>;
+ qcom,ports-lane-control = /bits/ 8 <0x01 0x00 0x00 0x00 0x00>;
+ qcom,ports-block-pack-mode = /bits/ 8 <0xFF 0x00 0x01 0xFF 0xFF>;
+ qcom,ports-hstart = /bits/ 8 <0xFF 0x03 0xFF 0xFF 0xFF>;
+ qcom,ports-hstop = /bits/ 8 <0xFF 0x06 0xFF 0xFF 0xFF>;
+ qcom,ports-block-group-count = /bits/ 8 <0xFF 0xFF 0xFF 0xFF 0x00>;
Why is this bit stuff in DT vs. in the device driver?

This is soundwire ports configuration and is followed in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi?h=v5.17-rc2


+
+ #sound-dai-cells = <1>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+ };
+
+ txmacro: txmacro@3220000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&tx_swr_active>;
+ compatible = "qcom,sc7280-lpass-tx-macro";
+ reg = <0 0x3220000 0 0x1000>;
+
+ clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+ <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+ <&vamacro>;
Please align the clocks here so the < starts on the same column.
Okay.

+ clock-names = "mclk", "npl", "fsgen";
+
+ #clock-cells = <0>;
+ clock-frequency = <9600000>;
+ clock-output-names = "mclk";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ #sound-dai-cells = <1>;
+ };
+
+ /* tx macro */
Do we need these comments? Please remove them as they're practically
useless.
Okay. Will remove it.

+ swr1: soundwire-controller@3230000 {
+ reg = <0 0x3230000 0 0x2000>;
+ compatible = "qcom,soundwire-v1.6.0";
+
+ interrupts-extended =
+ <&intc GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH>,
+ <&pdc 130 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "swr_master_irq", "swr_wake_irq";
+ clocks = <&txmacro>;
+ clock-names = "iface";
+ label = "TX";
+
+ qcom,din-ports = <3>;
+ qcom,dout-ports = <0>;
+ qcom,swrm-hctl-reg = <0x032a90a8>;
This looks like some common register location for soundwire oops bits.
Is it another device? Or maybe it's a common register base that needs to
be a syscon?
We are in discussion on this, how to handle it appropriately. Will change accordingly after design concluded.

+
+ qcom,ports-sinterval-low = /bits/ 8 <0x01 0x03 0x03>;
+ qcom,ports-offset1 = /bits/ 8 <0x01 0x00 0x02>;
+ qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x00>;
+ qcom,ports-hstart = /bits/ 8 <0xFF 0xFF 0xFF>;
+ qcom,ports-hstop = /bits/ 8 <0xFF 0xFF 0xFF>;
+ qcom,ports-word-length = /bits/ 8 <0xFF 0x0 0xFF>;
+ qcom,ports-block-pack-mode = /bits/ 8 <0xFF 0xFF 0xFF>;
+ qcom,ports-block-group-count = /bits/ 8 <0xFF 0xFF 0xFF>;
+ qcom,ports-lane-control = /bits/ 8 <0x00 0x01 0x00>;
+ qcom,port-offset = <1>;
+
+ #sound-dai-cells = <1>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+ };
+
+ vamacro: codec@3370000 {
+ compatible = "qcom,sc7280-lpass-va-macro";
+ pinctrl-0 = <&dmic01_active>;
+ pinctrl-names = "default";
+
+ vdd-micb-supply = <&vreg_bob>;
The supplies need to be set in the board file, no the soc file as they
can be different depending on what the hardware engineer wires to the
pins on the SoC.
Okay. Will move it to board specific file.

+ reg = <0 0x3370000 0 0x1000>;
+ clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+ <&lpass_audiocc LPASS_AUDIO_CC_RX_MCLK_CLK>;
+ clock-names = "mclk", "macro";
+
+ #clock-cells = <0>;
+ clock-frequency = <9600000>;
+ clock-output-names = "fsgen";
+ #sound-dai-cells = <1>;
+ };
+
lpass_tlmm: pinctrl@33c0000 {
compatible = "qcom,sc7280-lpass-lpi-pinctrl";
reg = <0 0x33c0000 0x0 0x20000>,
--
2.7.4