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

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

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