Add device nodes for most of the sound support - WSA883x smart speakers,
WCD9395 audio codec (headset) and sound card - which allows sound
playback via speakers and recording via DMIC microphones. Changes bring
necessary foundation for headset playback/recording via USB, but that
part is not yet ready.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 214 ++++++++++++++++++++++++++++++++
1 file changed, 214 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
index 72f081a890dfe49bfbee5e91b9e51da53b9d8baf..47645f15e6e327620840de8c3e03105540faf9cc 100644
--- a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
@@ -29,6 +29,32 @@ aliases {
serial0 = &uart7;
};
+ wcd939x: audio-codec {
+ compatible = "qcom,wcd9395-codec", "qcom,wcd9390-codec";
+
+ pinctrl-0 = <&wcd_default>;
+ pinctrl-names = "default";
+
+ 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>;
+ qcom,rx-device = <&wcd_rx>;
+ qcom,tx-device = <&wcd_tx>;
+
+ reset-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
+
+ vdd-buck-supply = <&vreg_l15b_1p8>;
+ vdd-rxtx-supply = <&vreg_l15b_1p8>;
+ vdd-io-supply = <&vreg_l15b_1p8>;
+ vdd-mic-bias-supply = <&vreg_bob1>;
+
+ #sound-dai-cells = <1>;
+ };
+
chosen {
stdout-path = "serial0:115200n8";
};
@@ -81,6 +107,89 @@ key-volume-up {
};
};
+ sound {
+ compatible = "qcom,sm8750-sndcard", "qcom,sm8450-sndcard";
+ model = "SM8750-MTP";
+ audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
+ "SpkrRight IN", "WSA_SPK2 OUT",
+ "IN1_HPHL", "HPHL_OUT",
+ "IN2_HPHR", "HPHR_OUT",
+ "AMIC2", "MIC BIAS2",
+ "VA DMIC0", "MIC BIAS3", /* MIC4 on schematics */
+ "VA DMIC1", "MIC BIAS3", /* MIC1 on schematics */
+ "VA DMIC2", "MIC BIAS1",
+ "VA DMIC3", "MIC BIAS1",
+ "VA DMIC0", "VA MIC BIAS3",
+ "VA DMIC1", "VA MIC BIAS3",
+ "VA DMIC2", "VA MIC BIAS1",
+ "VA DMIC3", "VA MIC BIAS1",
+ "TX SWR_INPUT1", "ADC2_OUTPUT";
+
+ wcd-playback-dai-link {
+ link-name = "WCD Playback";
+
+ cpu {
+ sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
+ };
+
+ codec {
+ sound-dai = <&wcd939x 0>, <&swr1 0>, <&lpass_rxmacro 0>;
+ };
+
+ platform {
+ sound-dai = <&q6apm>;
+ };
+ };
+
+ wcd-capture-dai-link {
+ link-name = "WCD Capture";
+
+ cpu {
+ sound-dai = <&q6apmbedai TX_CODEC_DMA_TX_3>;
+ };
+
+ codec {
+ sound-dai = <&wcd939x 1>, <&swr2 0>, <&lpass_txmacro 0>;
+ };
+
+ platform {
+ sound-dai = <&q6apm>;
+ };
+ };
+
+ wsa-dai-link {
+ link-name = "WSA Playback";
+
+ cpu {
+ sound-dai = <&q6apmbedai WSA_CODEC_DMA_RX_0>;
+ };
+
+ codec {
+ sound-dai = <&left_spkr>, <&right_spkr>, <&swr0 0>, <&lpass_wsamacro 0>;
+ };
+
+ platform {
+ sound-dai = <&q6apm>;
+ };
+ };
+
+ va-dai-link {
+ link-name = "VA Capture";
+
+ cpu {
+ sound-dai = <&q6apmbedai VA_CODEC_DMA_TX_0>;
+ };
+
+ codec {
+ sound-dai = <&lpass_vamacro 0>;
+ };
+
+ platform {
+ sound-dai = <&q6apm>;
+ };
+ };
+ };
+
vph_pwr: vph-pwr-regulator {
compatible = "regulator-fixed";
@@ -702,6 +811,14 @@ vreg_l7n_3p3: ldo7 {
};
};
+&lpass_vamacro {
+ pinctrl-0 = <&dmic01_default>, <&dmic23_default>;
+ pinctrl-names = "default";
+
+ vdd-micb-supply = <&vreg_l1b_1p8>;
+ qcom,dmic-sample-rate = <4800000>;
+};
+
&pm8550_flash {
status = "okay";
@@ -806,6 +923,74 @@ &remoteproc_mpss {
status = "fail";
};
+&swr0 {
+ status = "okay";
+
+ /* WSA883x, left/front speaker */
+ left_spkr: speaker@0,1 {
+ compatible = "sdw10217020200";
+ reg = <0 1>;
+ pinctrl-0 = <&spkr_0_sd_n_active>;
+ pinctrl-names = "default";
+ powerdown-gpios = <&lpass_tlmm 17 GPIO_ACTIVE_LOW>;
+ #sound-dai-cells = <0>;
+ sound-name-prefix = "SpkrLeft";
+ #thermal-sensor-cells = <0>;
+ vdd-supply = <&vreg_l15b_1p8>;
+ };
+
+ /* WSA883x, right/back speaker */
+ right_spkr: speaker@0,2 {
+ compatible = "sdw10217020200";
+ reg = <0 2>;
+ pinctrl-0 = <&spkr_1_sd_n_active>;
+ pinctrl-names = "default";
+ powerdown-gpios = <&lpass_tlmm 18 GPIO_ACTIVE_LOW>;
+ #sound-dai-cells = <0>;
+ sound-name-prefix = "SpkrRight";
+ #thermal-sensor-cells = <0>;
+ vdd-supply = <&vreg_l15b_1p8>;
+ };
+};
+
+&swr1 {
+ status = "okay";
+
+ /* WCD9395 RX */
+ wcd_rx: codec@0,4 {
+ compatible = "sdw20217010e00";
+ reg = <0 4>;
+
+ /*
+ * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R)
+ * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH)
+ * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R)
+ * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO)
+ * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R)
+ * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R)
+ */
+ qcom,rx-port-mapping = <1 2 3 4 5 9>;
+ };
+};
+
+&swr2 {
+ status = "okay";
+
+ /* WCD9395 TX */
+ wcd_tx: codec@0,3 {
+ compatible = "sdw20217010e00";
+ reg = <0 3>;
+
+ /*
+ * WCD9395 TX Port 1 (ADC1,2,3,4) <=> SWR2 Port 2 (TX SWR_INPUT 0,1,2,3)
+ * WCD9395 TX Port 2 (ADC3,4 & DMIC0,1) <=> SWR2 Port 2 (TX SWR_INPUT 0,1,2,3)
+ * WCD9395 TX Port 3 (DMIC0,1,2,3 & MBHC) <=> SWR2 Port 3 (TX SWR_INPUT 4,5,6,7)
+ * WCD9395 TX Port 4 (DMIC4,5,6,7) <=> SWR2 Port 4 (TX SWR_INPUT 8,9,10,11)
+ */
+ qcom,tx-port-mapping = <2 2 3 4>;
+ };
+};
+
&tlmm {
/* reserved for secure world */
gpio-reserved-ranges = <36 4>, <74 1>;
@@ -814,3 +999,32 @@ &tlmm {
&uart7 {
status = "okay";
};
+
+/* Pinctrl */
+&lpass_tlmm {
+ spkr_0_sd_n_active: spkr-0-sd-n-active-state {
+ pins = "gpio17";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ output-low;
+ };
+
+ spkr_1_sd_n_active: spkr-1-sd-n-active-state {
+ pins = "gpio18";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ output-low;
+ };
+};
+
+&tlmm {
+ wcd_default: wcd-reset-n-active-state {
+ pins = "gpio101";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ output-low;
+ };
+};
--
2.45.2On 4/24/25 11:40 AM, Krzysztof Kozlowski wrote:
> Add device nodes for most of the sound support - WSA883x smart speakers,
> WCD9395 audio codec (headset) and sound card - which allows sound
> playback via speakers and recording via DMIC microphones. Changes bring
> necessary foundation for headset playback/recording via USB, but that
> part is not yet ready.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
[...]
> + sound {
> + compatible = "qcom,sm8750-sndcard", "qcom,sm8450-sndcard";
> + model = "SM8750-MTP";
> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
> + "SpkrRight IN", "WSA_SPK2 OUT",
> + "IN1_HPHL", "HPHL_OUT",
> + "IN2_HPHR", "HPHR_OUT",
> + "AMIC2", "MIC BIAS2",
> + "VA DMIC0", "MIC BIAS3", /* MIC4 on schematics */
> + "VA DMIC1", "MIC BIAS3", /* MIC1 on schematics */
Is this a mistake in what the codec driver exposes, or just a fumble
in numbering $somewhere?
> + "VA DMIC2", "MIC BIAS1",
> + "VA DMIC3", "MIC BIAS1",
> + "VA DMIC0", "VA MIC BIAS3",
> + "VA DMIC1", "VA MIC BIAS3",
> + "VA DMIC2", "VA MIC BIAS1",
> + "VA DMIC3", "VA MIC BIAS1",
> + "TX SWR_INPUT1", "ADC2_OUTPUT";
> +
> + wcd-playback-dai-link {
> + link-name = "WCD Playback";
> +
> + cpu {
> + sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
> + };
> +
> + codec {
'co'dec < 'cp'u
[...]
> + /*
> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R)
> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH)
> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R)
> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO)
> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R)
> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R)
> + */
> + qcom,rx-port-mapping = <1 2 3 4 5 9>;
Does this deserve some dt-bindings constants?
Konrad
On 25/04/2025 11:30, Konrad Dybcio wrote:
> On 4/24/25 11:40 AM, Krzysztof Kozlowski wrote:
>> Add device nodes for most of the sound support - WSA883x smart speakers,
>> WCD9395 audio codec (headset) and sound card - which allows sound
>> playback via speakers and recording via DMIC microphones. Changes bring
>> necessary foundation for headset playback/recording via USB, but that
>> part is not yet ready.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>
> [...]
>
>> + sound {
>> + compatible = "qcom,sm8750-sndcard", "qcom,sm8450-sndcard";
>> + model = "SM8750-MTP";
>> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
>> + "SpkrRight IN", "WSA_SPK2 OUT",
>> + "IN1_HPHL", "HPHL_OUT",
>> + "IN2_HPHR", "HPHR_OUT",
>> + "AMIC2", "MIC BIAS2",
>> + "VA DMIC0", "MIC BIAS3", /* MIC4 on schematics */
>> + "VA DMIC1", "MIC BIAS3", /* MIC1 on schematics */
>
> Is this a mistake in what the codec driver exposes, or just a fumble
> in numbering $somewhere?
Which mistake? MIC4? Schematics call name things differently. They
always were, so to make it clear for people without schematics I wrote
which MIC it actually is.
>
>> + "VA DMIC2", "MIC BIAS1",
>> + "VA DMIC3", "MIC BIAS1",
>> + "VA DMIC0", "VA MIC BIAS3",
>> + "VA DMIC1", "VA MIC BIAS3",
>> + "VA DMIC2", "VA MIC BIAS1",
>> + "VA DMIC3", "VA MIC BIAS1",
>> + "TX SWR_INPUT1", "ADC2_OUTPUT";
>> +
>> + wcd-playback-dai-link {
>> + link-name = "WCD Playback";
>> +
>> + cpu {
>> + sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
>> + };
>> +
>> + codec {
>
> 'co'dec < 'cp'u
>
> [...]
That was the convention so far, but we can start a new one, sure. Just
ask the same all other patch contributors, because each of them will be
copying old code, which means cpu->codec->platform
>
>> + /*
>> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R)
>> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH)
>> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R)
>> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO)
>> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R)
>> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R)
>> + */
>> + qcom,rx-port-mapping = <1 2 3 4 5 9>;
>
> Does this deserve some dt-bindings constants?
No, because these are hardware details/constants. Drivers do not use them.
Best regards,
Krzysztof
On 4/28/25 4:41 PM, Krzysztof Kozlowski wrote:
> On 25/04/2025 11:30, Konrad Dybcio wrote:
>> On 4/24/25 11:40 AM, Krzysztof Kozlowski wrote:
>>> Add device nodes for most of the sound support - WSA883x smart speakers,
>>> WCD9395 audio codec (headset) and sound card - which allows sound
>>> playback via speakers and recording via DMIC microphones. Changes bring
>>> necessary foundation for headset playback/recording via USB, but that
>>> part is not yet ready.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>
>> [...]
>>
>>> + sound {
>>> + compatible = "qcom,sm8750-sndcard", "qcom,sm8450-sndcard";
>>> + model = "SM8750-MTP";
>>> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
>>> + "SpkrRight IN", "WSA_SPK2 OUT",
>>> + "IN1_HPHL", "HPHL_OUT",
>>> + "IN2_HPHR", "HPHR_OUT",
>>> + "AMIC2", "MIC BIAS2",
>>> + "VA DMIC0", "MIC BIAS3", /* MIC4 on schematics */
>>> + "VA DMIC1", "MIC BIAS3", /* MIC1 on schematics */
>>
>> Is this a mistake in what the codec driver exposes, or just a fumble
>> in numbering $somewhere?
>
> Which mistake? MIC4? Schematics call name things differently. They
> always were, so to make it clear for people without schematics I wrote
> which MIC it actually is.
I'm not sure how to parse your response
are you saying that there are MIC[0..4] that are/may be connected
to different codec ports, and that the MIC4/1 lines are plumbed to
VA DMIC0/1 respectively?
I think I got confused about the MIC BIAS3 going to both and none
matching the index, but perhaps that's just because it comes from
the WCD (which is the third piece of hw involved beyond VA and the
mic itself)
>
>>
>>> + "VA DMIC2", "MIC BIAS1",
>>> + "VA DMIC3", "MIC BIAS1",
>>> + "VA DMIC0", "VA MIC BIAS3",
>>> + "VA DMIC1", "VA MIC BIAS3",
>>> + "VA DMIC2", "VA MIC BIAS1",
>>> + "VA DMIC3", "VA MIC BIAS1",
>>> + "TX SWR_INPUT1", "ADC2_OUTPUT";
>>> +
>>> + wcd-playback-dai-link {
>>> + link-name = "WCD Playback";
>>> +
>>> + cpu {
>>> + sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
>>> + };
>>> +
>>> + codec {
>>
>> 'co'dec < 'cp'u
>>
>> [...]
>
> That was the convention so far, but we can start a new one, sure. Just
> ask the same all other patch contributors, because each of them will be
> copying old code, which means cpu->codec->platform
I've been doing just that for the past couple weeks indeed
>>> + /*
>>> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R)
>>> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH)
>>> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R)
>>> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO)
>>> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R)
>>> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R)
>>> + */
>>> + qcom,rx-port-mapping = <1 2 3 4 5 9>;
>>
>> Does this deserve some dt-bindings constants?
>
> No, because these are hardware details/constants. Drivers do not use them.
I'd argue it makes sense here - it makes more sense to pass meaningfully
named constants to the driver, rather than blobs with a comment
Konrad
On 29/04/2025 21:11, Konrad Dybcio wrote:
> On 4/28/25 4:41 PM, Krzysztof Kozlowski wrote:
>> On 25/04/2025 11:30, Konrad Dybcio wrote:
>>> On 4/24/25 11:40 AM, Krzysztof Kozlowski wrote:
>>>> Add device nodes for most of the sound support - WSA883x smart speakers,
>>>> WCD9395 audio codec (headset) and sound card - which allows sound
>>>> playback via speakers and recording via DMIC microphones. Changes bring
>>>> necessary foundation for headset playback/recording via USB, but that
>>>> part is not yet ready.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>
>>> [...]
>>>
>>>> + sound {
>>>> + compatible = "qcom,sm8750-sndcard", "qcom,sm8450-sndcard";
>>>> + model = "SM8750-MTP";
>>>> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
>>>> + "SpkrRight IN", "WSA_SPK2 OUT",
>>>> + "IN1_HPHL", "HPHL_OUT",
>>>> + "IN2_HPHR", "HPHR_OUT",
>>>> + "AMIC2", "MIC BIAS2",
>>>> + "VA DMIC0", "MIC BIAS3", /* MIC4 on schematics */
>>>> + "VA DMIC1", "MIC BIAS3", /* MIC1 on schematics */
>>>
>>> Is this a mistake in what the codec driver exposes, or just a fumble
>>> in numbering $somewhere?
>>
>> Which mistake? MIC4? Schematics call name things differently. They
>> always were, so to make it clear for people without schematics I wrote
>> which MIC it actually is.
>
> I'm not sure how to parse your response
>
> are you saying that there are MIC[0..4] that are/may be connected
> to different codec ports, and that the MIC4/1 lines are plumbed to
> VA DMIC0/1 respectively?
Yes, as always. Nothing weird here.
>
> I think I got confused about the MIC BIAS3 going to both and none
What is both and none?
> matching the index, but perhaps that's just because it comes from
> the WCD (which is the third piece of hw involved beyond VA and the
> mic itself)
Again, what is the mistake you are pointing here?
>
>>
>>>
>>>> + "VA DMIC2", "MIC BIAS1",
>>>> + "VA DMIC3", "MIC BIAS1",
>>>> + "VA DMIC0", "VA MIC BIAS3",
>>>> + "VA DMIC1", "VA MIC BIAS3",
>>>> + "VA DMIC2", "VA MIC BIAS1",
>>>> + "VA DMIC3", "VA MIC BIAS1",
>>>> + "TX SWR_INPUT1", "ADC2_OUTPUT";
>>>> +
>>>> + wcd-playback-dai-link {
>>>> + link-name = "WCD Playback";
>>>> +
>>>> + cpu {
>>>> + sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
>>>> + };
>>>> +
>>>> + codec {
>>>
>>> 'co'dec < 'cp'u
>>>
>>> [...]
>>
>> That was the convention so far, but we can start a new one, sure. Just
>> ask the same all other patch contributors, because each of them will be
>> copying old code, which means cpu->codec->platform
>
> I've been doing just that for the past couple weeks indeed
>
>>>> + /*
>>>> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R)
>>>> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH)
>>>> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R)
>>>> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO)
>>>> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R)
>>>> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R)
>>>> + */
>>>> + qcom,rx-port-mapping = <1 2 3 4 5 9>;
>>>
>>> Does this deserve some dt-bindings constants?
>>
>> No, because these are hardware details/constants. Drivers do not use them.
>
> I'd argue it makes sense here - it makes more sense to pass meaningfully
> named constants to the driver, rather than blobs with a comment
Sense of what? You want to make it a binding then answer what does it
bind, what part of ABI for driver is here a binding (answer none:
because driver does not use it)?
Best regards,
Krzysztof
On 4/30/25 8:19 AM, Krzysztof Kozlowski wrote:
> On 29/04/2025 21:11, Konrad Dybcio wrote:
>> On 4/28/25 4:41 PM, Krzysztof Kozlowski wrote:
>>> On 25/04/2025 11:30, Konrad Dybcio wrote:
>>>> On 4/24/25 11:40 AM, Krzysztof Kozlowski wrote:
>>>>> Add device nodes for most of the sound support - WSA883x smart speakers,
>>>>> WCD9395 audio codec (headset) and sound card - which allows sound
>>>>> playback via speakers and recording via DMIC microphones. Changes bring
>>>>> necessary foundation for headset playback/recording via USB, but that
>>>>> part is not yet ready.
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> + sound {
>>>>> + compatible = "qcom,sm8750-sndcard", "qcom,sm8450-sndcard";
>>>>> + model = "SM8750-MTP";
>>>>> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
>>>>> + "SpkrRight IN", "WSA_SPK2 OUT",
>>>>> + "IN1_HPHL", "HPHL_OUT",
>>>>> + "IN2_HPHR", "HPHR_OUT",
>>>>> + "AMIC2", "MIC BIAS2",
>>>>> + "VA DMIC0", "MIC BIAS3", /* MIC4 on schematics */
>>>>> + "VA DMIC1", "MIC BIAS3", /* MIC1 on schematics */
>>>>
>>>> Is this a mistake in what the codec driver exposes, or just a fumble
>>>> in numbering $somewhere?
>>>
>>> Which mistake? MIC4? Schematics call name things differently. They
>>> always were, so to make it clear for people without schematics I wrote
>>> which MIC it actually is.
>>
>> I'm not sure how to parse your response
>>
>> are you saying that there are MIC[0..4] that are/may be connected
>> to different codec ports, and that the MIC4/1 lines are plumbed to
>> VA DMIC0/1 respectively?
>
> Yes, as always. Nothing weird here.
>
>>
>> I think I got confused about the MIC BIAS3 going to both and none
>
> What is both and none?
missing Oxford comma I suppose.. MIC BIAS3 goes to both MIC4 and MIC1
and neither of them has a '3' in the name. I was wondering whether
that's intentional.
>
>> matching the index, but perhaps that's just because it comes from
>> the WCD (which is the third piece of hw involved beyond VA and the
>> mic itself)
>
> Again, what is the mistake you are pointing here?
I'm not necessarily saying this is a mistake, just thinking out loud
how I understand the non-obvious plumbing
[...]
>>>>> + /*
>>>>> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R)
>>>>> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH)
>>>>> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R)
>>>>> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO)
>>>>> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R)
>>>>> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R)
>>>>> + */
>>>>> + qcom,rx-port-mapping = <1 2 3 4 5 9>;
>>>>
>>>> Does this deserve some dt-bindings constants?
>>>
>>> No, because these are hardware details/constants. Drivers do not use them.
>>
>> I'd argue it makes sense here - it makes more sense to pass meaningfully
>> named constants to the driver, rather than blobs with a comment
>
> Sense of what? You want to make it a binding then answer what does it
> bind, what part of ABI for driver is here a binding (answer none:
> because driver does not use it)?
Sense of the magic numbers that otherwise require a comment.
dt-bindings don't exclusively contain enums-turned-defines that are
indices of C arrays - some contain various forms of hardware ABI, be
it register addresses, or names for magic values
Konrad
On 30/04/2025 12:48, Konrad Dybcio wrote: >>>>>> + /* >>>>>> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R) >>>>>> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH) >>>>>> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R) >>>>>> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO) >>>>>> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R) >>>>>> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R) >>>>>> + */ >>>>>> + qcom,rx-port-mapping = <1 2 3 4 5 9>; >>>>> >>>>> Does this deserve some dt-bindings constants? >>>> >>>> No, because these are hardware details/constants. Drivers do not use them. >>> >>> I'd argue it makes sense here - it makes more sense to pass meaningfully >>> named constants to the driver, rather than blobs with a comment >> >> Sense of what? You want to make it a binding then answer what does it >> bind, what part of ABI for driver is here a binding (answer none: >> because driver does not use it)? > > Sense of the magic numbers that otherwise require a comment. > > dt-bindings don't exclusively contain enums-turned-defines that are No, they don't. Best regards, Krzysztof
On 30/04/2025 12:48, Konrad Dybcio wrote:
> On 4/30/25 8:19 AM, Krzysztof Kozlowski wrote:
>> On 29/04/2025 21:11, Konrad Dybcio wrote:
>>> On 4/28/25 4:41 PM, Krzysztof Kozlowski wrote:
>>>> On 25/04/2025 11:30, Konrad Dybcio wrote:
>>>>> On 4/24/25 11:40 AM, Krzysztof Kozlowski wrote:
>>>>>> Add device nodes for most of the sound support - WSA883x smart speakers,
>>>>>> WCD9395 audio codec (headset) and sound card - which allows sound
>>>>>> playback via speakers and recording via DMIC microphones. Changes bring
>>>>>> necessary foundation for headset playback/recording via USB, but that
>>>>>> part is not yet ready.
>>>>>>
>>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>> ---
>>>>>
>>>>> [...]
>>>>>
>>>>>> + sound {
>>>>>> + compatible = "qcom,sm8750-sndcard", "qcom,sm8450-sndcard";
>>>>>> + model = "SM8750-MTP";
>>>>>> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
>>>>>> + "SpkrRight IN", "WSA_SPK2 OUT",
>>>>>> + "IN1_HPHL", "HPHL_OUT",
>>>>>> + "IN2_HPHR", "HPHR_OUT",
>>>>>> + "AMIC2", "MIC BIAS2",
>>>>>> + "VA DMIC0", "MIC BIAS3", /* MIC4 on schematics */
>>>>>> + "VA DMIC1", "MIC BIAS3", /* MIC1 on schematics */
>>>>>
>>>>> Is this a mistake in what the codec driver exposes, or just a fumble
>>>>> in numbering $somewhere?
>>>>
>>>> Which mistake? MIC4? Schematics call name things differently. They
>>>> always were, so to make it clear for people without schematics I wrote
>>>> which MIC it actually is.
>>>
>>> I'm not sure how to parse your response
>>>
>>> are you saying that there are MIC[0..4] that are/may be connected
>>> to different codec ports, and that the MIC4/1 lines are plumbed to
>>> VA DMIC0/1 respectively?
>>
>> Yes, as always. Nothing weird here.
>>
>>>
>>> I think I got confused about the MIC BIAS3 going to both and none
>>
>> What is both and none?
>
> missing Oxford comma I suppose.. MIC BIAS3 goes to both MIC4 and MIC1
> and neither of them has a '3' in the name. I was wondering whether
> that's intentional.
>
>>
>>> matching the index, but perhaps that's just because it comes from
>>> the WCD (which is the third piece of hw involved beyond VA and the
>>> mic itself)
>>
>> Again, what is the mistake you are pointing here?
>
> I'm not necessarily saying this is a mistake, just thinking out loud
> how I understand the non-obvious plumbing
>
> [...]
>
>>>>>> + /*
>>>>>> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R)
>>>>>> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH)
>>>>>> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R)
>>>>>> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO)
>>>>>> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R)
>>>>>> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R)
>>>>>> + */
>>>>>> + qcom,rx-port-mapping = <1 2 3 4 5 9>;
>>>>>
>>>>> Does this deserve some dt-bindings constants?
>>>>
>>>> No, because these are hardware details/constants. Drivers do not use them.
>>>
>>> I'd argue it makes sense here - it makes more sense to pass meaningfully
>>> named constants to the driver, rather than blobs with a comment
>>
>> Sense of what? You want to make it a binding then answer what does it
>> bind, what part of ABI for driver is here a binding (answer none:
>> because driver does not use it)?
>
> Sense of the magic numbers that otherwise require a comment.
There's no magic numbers, index of qcom,rx-port-mapping is the RX port,
value is the SWR1 port index. As the property name says, it maps RX ports.
The comment is here to understand why we map as-is, and what the ports are
used for, but for the soundwire perspective only the numbers matters.
>
> dt-bindings don't exclusively contain enums-turned-defines that are
> indices of C arrays - some contain various forms of hardware ABI, be
> it register addresses, or names for magic values
>
> Konrad
>
On 4/30/25 1:07 PM, neil.armstrong@linaro.org wrote: > On 30/04/2025 12:48, Konrad Dybcio wrote: >> On 4/30/25 8:19 AM, Krzysztof Kozlowski wrote: >>> On 29/04/2025 21:11, Konrad Dybcio wrote: >>>> On 4/28/25 4:41 PM, Krzysztof Kozlowski wrote: >>>>> On 25/04/2025 11:30, Konrad Dybcio wrote: >>>>>> On 4/24/25 11:40 AM, Krzysztof Kozlowski wrote: >>>>>>> Add device nodes for most of the sound support - WSA883x smart speakers, >>>>>>> WCD9395 audio codec (headset) and sound card - which allows sound >>>>>>> playback via speakers and recording via DMIC microphones. Changes bring >>>>>>> necessary foundation for headset playback/recording via USB, but that >>>>>>> part is not yet ready. >>>>>>> >>>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>>> --- [...] >>>>>>> + /* >>>>>>> + * WCD9395 RX Port 1 (HPH_L/R) <=> SWR1 Port 1 (HPH_L/R) >>>>>>> + * WCD9395 RX Port 2 (CLSH) <=> SWR1 Port 2 (CLSH) >>>>>>> + * WCD9395 RX Port 3 (COMP_L/R) <=> SWR1 Port 3 (COMP_L/R) >>>>>>> + * WCD9395 RX Port 4 (LO) <=> SWR1 Port 4 (LO) >>>>>>> + * WCD9395 RX Port 5 (DSD_L/R) <=> SWR1 Port 5 (DSD_L/R) >>>>>>> + * WCD9395 RX Port 6 (HIFI_PCM_L/R) <=> SWR1 Port 9 (HIFI_PCM_L/R) >>>>>>> + */ >>>>>>> + qcom,rx-port-mapping = <1 2 3 4 5 9>; >>>>>> >>>>>> Does this deserve some dt-bindings constants? >>>>> >>>>> No, because these are hardware details/constants. Drivers do not use them. >>>> >>>> I'd argue it makes sense here - it makes more sense to pass meaningfully >>>> named constants to the driver, rather than blobs with a comment >>> >>> Sense of what? You want to make it a binding then answer what does it >>> bind, what part of ABI for driver is here a binding (answer none: >>> because driver does not use it)? >> >> Sense of the magic numbers that otherwise require a comment. > > There's no magic numbers, index of qcom,rx-port-mapping is the RX port, > value is the SWR1 port index. As the property name says, it maps RX ports. > > The comment is here to understand why we map as-is, and what the ports are > used for, but for the soundwire perspective only the numbers matters. OK so it's the indices on the WCD side that are hardwired, IIUC. So perhaps that comment could be included in qcom,wcd939x-sdw.yaml under items: for qcom,rx-port-mapping Konrad
© 2016 - 2026 Red Hat, Inc.