Re: [PATCH 1/3] dt-bindings: sound: add bindings for pm4125 audio codec

[Krzysztof Kozlowski]

On 28/06/2025 18:41, Alexey Klimov wrote:
> 
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +
>>> +            audio-codec@f000 {
>>> +                compatible = "qcom,pm4125-codec";
>>> +                reg = <0xf000>;
>>> +                vdd-io-supply = <&pm4125_l15>;
>>> +                vdd-cp-supply = <&pm4125_s4>;
>>> +                vdd-pa-vpos-supply = <&pm4125_s4>;
>>> +                vdd-mic-bias-supply = <&pm4125_l22>;
>>> +                qcom,micbias1-microvolt = <1800000>;
>>> +                qcom,micbias2-microvolt = <1800000>;
>>> +                qcom,micbias3-microvolt = <1800000>;
>>> +                qcom,rx-device = <&pm4125_rx>;
>>> +                qcom,tx-device = <&pm4125_tx>;
>>> +                #sound-dai-cells = <1>;
>>> +            };
>>> +        };
>>> +    };
>>> +
>>> +    /* ... */
>>> +
>>> +    soundwire@a610000 {
>>
>> Drop this and next one.
> 
> The audio-codec node supposed to have qcom,{rx,tx}-device properties.
> If I'll drop it then the example doesn't compile well unless I am missing
> something?

What did you drop and what did I ask to drop?

> 
> For example when I removed soundwire tx node completely and dropped
> qcom,tx-device then:

I did not ask to drop qcom,tx-device.

...

> 
>>> +  The audio codec IC found on Qualcomm PM4125/PM2250 PMICs.
>>> +  It has RX and TX Soundwire slave devices. This bindings is for the
>>> +  slave devices.
>>
>> Last sentence is redundant and makes no sense. Codec has only slave
>> devices, so how this can be anything else than for slave devices?
> 
> This came from other similar files that describe bindings for child codec nodes
> of soundwire nodes. For example from qcom,wcd937x-sdw.yaml.
> Should this be rephrased to "This bindings is for the soundwire slave devices." ?

You just pasted the same, so I don't get how you want to rephrase into
the same sentence.

> 
Best regards,
Krzysztof