Re: [PATCH v2 1/8] ASoC: dt-bindings: wcd937x: add bindings for wcd937x

Mohammad Rafi Shaik
2024-04-22T06:56:25Z

On 4/17/2024 9:21 PM, Krzysztof Kozlowski wrote:
> On 16/04/2024 08:35, Mohammad Rafi Shaik wrote:
>> From: Prasad Kumpatla <quic_pkumpatl@quicinc.com>
>>
>> Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC
>> connected over SoundWire. This device has two SoundWire device RX and
>> TX respectively, supporting 3 x ADCs, ClassH, Ear, Aux PA, 2xHPH,
>> 6 DMICs, MBHC.
>>
>> Co-developed-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com>
>> ---
>> .../bindings/sound/qcom,wcd937x.yaml | 119 ++++++++++++++++++
>> 1 file changed, 119 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
>> new file mode 100644
>> index 000000000000..b45f823af04f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
>
> No wildcards. This is wcd9370, right?
>
>> @@ -0,0 +1,119 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/qcom,wcd937x.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm WCD9370 Audio Codec
>> +
>> +maintainers:
>> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> +
>> +description: |
>
> Do not need '|' unless you need to preserve formatting.

ACK

will remove '|' in next version patch set.

>
>> + Qualcomm WCD9370 Codec is a standalone Hi-Fi audio codec IC.
>> + It has RX and TX Soundwire slave devices.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - qcom,wcd9370-codec
>
> Can it be anything else than codec?
>
>> +
>> + reset-gpios:
>> + description: GPIO spec for reset line to use
>> + maxItems: 1
>> +
>> + vdd-buck-supply:
>> + description: A reference to the 1.8V buck supply
>> +
>> + vdd-rxtx-supply:
>> + description: A reference to the 1.8V rx supply
>> +
>> + vdd-px-supply:
>> + description: A reference to the 1.8V I/O supply
>> +
>> + vdd-mic-bias-supply:
>> + description: A reference to the 3.8V mic bias supply
>> +
>> + qcom,tx-device:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: A reference to Soundwire tx device phandle
>
> "A reference to phandle" is not correct. Phandle is the reference.
> Anyway, both are redundant.
>

ACK,

>>
>> + qcom,rx-device:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: A reference to Soundwire rx device phandle
>> +
>> + qcom,micbias1-microvolt:
>> + description: micbias1 voltage
>> + minimum: 1800000
>> + maximum: 2850000
>> +
>> + qcom,micbias2-microvolt:
>> + description: micbias2 voltage
>> + minimum: 1800000
>> + maximum: 2850000
>> +
>> + qcom,micbias3-microvolt:
>> + description: micbias3 voltage
>> + minimum: 1800000
>> + maximum: 2850000
>> +
>> + '#sound-dai-cells':
>> + const: 1
>> +
>> +required:
>> + - compatible
>> + - reset-gpios
>> + - qcom,tx-device
>> + - qcom,rx-device
>> + - qcom,micbias1-microvolt
>> + - qcom,micbias2-microvolt
>> + - qcom,micbias3-microvolt
>> + - "#sound-dai-cells"
>> +
>> +unevaluatedProperties: false
>
> This does not make sense without ref to dai-common, so this should point
> you to missing ref.

ACK,

Will add missing dai-common reference.
>
>> +
>> +examples:
>> + - |
>> + codec {
>> + compatible = "qcom,wcd9370-codec";
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&wcd_reset_n>;
>> + pinctrl-1 = <&wcd_reset_n_sleep>;
>> + reset-gpios = <&tlmm 83 0>;
>> + vdd-buck-supply = <&vreg_l17b_1p8>;
>> + vdd-rxtx-supply = <&vreg_l18b_1p8>;
>> + vdd-px-supply = <&vreg_l18b_1p8>;
>> + vdd-mic-bias-supply = <&vreg_bob>;
>> + qcom,micbias1-microvolt = <1800000>;
>> + qcom,micbias2-microvolt = <1800000>;
>> + qcom,micbias3-microvolt = <1800000>;
>> + qcom,rx-device = <&wcd937x_rx>;
>> + qcom,tx-device = <&wcd937x_tx>;
>> + #sound-dai-cells = <1>;
>> + };
>> +
>> + /* ... */
>> +
>> + soundwire@3210000 {
>> + #address-cells = <2>;
>
> Drop this and other nodes.
>
> Best regards,
> Krzysztof

ACK,

will use the qcom,wcd93xx-common.yaml file and will drop the common
duplicate nodes

Thanks and Regards,
Rafi
>