linux-kernel - Re: [PATCH 2/2] arm64: dts: qcom: sm8750-mtp: Add sound (speakers, headset codec, dmics)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9b6c5f67-0bbc-490f-9982-4e28218aa6eb@oss.qualcomm.com>
Date: Tue, 29 Apr 2025 21:11:46 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8750-mtp: Add sound (speakers,
 headset codec, dmics)

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@...aro.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