Received-SPF: pass (google.com: domain of linux-kernel+bounces-152809-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <0b2de111-b5b4-ebf8-c9c9-81e90e8d6039@quicinc.com>
Date: Mon, 22 Apr 2024 12:25:46 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v2 1/8] ASoC: dt-bindings: wcd937x: add bindings for
 wcd937x
Content-Language: en-US
To: Krzysztof Kozlowski <krzk@kernel.org>,
        Srinivas Kandagatla
	<srinivas.kandagatla@linaro.org>,
        Banajit Goswami <bgoswami@quicinc.com>,
        Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>,
        "Rob
 Herring" <robh@kernel.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        "Conor
 Dooley" <conor+dt@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>, Takashi Iwai
	<tiwai@suse.com>
CC: <alsa-devel@alsa-project.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-sound@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_rohkumar@quicinc.com>,
        <quic_pkumpatl@quicinc.com>
References: <20240416063600.309747-1-quic_mohs@quicinc.com>
 <20240416063600.309747-2-quic_mohs@quicinc.com>
 <09410fa9-265e-4fb0-9814-35dee465dbec@kernel.org>
From: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
In-Reply-To: <09410fa9-265e-4fb0-9814-35dee465dbec@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

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
>