diff mbox series

[2/5] ASoC: dt-bindings: document WCD939x Audio Codec

Message ID 20231123-topic-sm8650-upstream-wcd939x-codec-v1-2-21d4ad9276de@linaro.org
State Superseded
Headers show
Series ASoC: codecs: add support for WCD939x Codec | expand

Commit Message

Neil Armstrong Nov. 23, 2023, 2:49 p.m. UTC
Document the Qualcomm WCD9390/WCD9395 Audio Codec and the
Soundwire slaves than can be found on Qualcomm SM8650 based platforms.

The Qualcomm WCD9390/WCD9395 Audio Codec communicates
with the host SoC over 2 Soundwire links to provide:
- 4 ADC inputs for up to 5 Analog Microphones
- 4 DMIC inputs for up to 8 Digital Microphones
- 4 Microphone BIAS
- Stereo Headphone output
- Mono EAR output
- MBHC engine for Headset Detection

The WCD9390/WCD9395 IC has separate I2C subsystem for USB-C
DP Altmode/Audio Accessory mode Mux which is not documented
here but requires port/endpoint graph and properties to handle
Altmode/Audio Accessory switch and orientation.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../bindings/sound/qcom,wcd939x-sdw.yaml           | 70 ++++++++++++++++
 .../devicetree/bindings/sound/qcom,wcd939x.yaml    | 93 ++++++++++++++++++++++
 2 files changed, 163 insertions(+)

Comments

Krzysztof Kozlowski Nov. 24, 2023, 8:33 a.m. UTC | #1
On 23/11/2023 15:49, Neil Armstrong wrote:

> +  Qualcomm WCD9390/WCD9395 Codec is a standalone Hi-Fi audio codec IC.
> +  It has RX and TX Soundwire slave devices.
> +  The WCD9390/WCD9395 IC has a functionally separate USB-C Mux subsystem
> +  accessible over an I2C interface.
> +  The Audio Headphone and Microphone data path between the Codec and the USB-C Mux
> +  subsystems are external to the IC, thus requiring DT port-endpoint graph description
> +  to handle USB-C altmode & orientation switching for Audio Accessory Mode.
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +  - $ref: qcom,wcd93xx-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,wcd9390-codec
> +      - qcom,wcd9395-codec

9395 should be compatible with 9390, so please express it with a list
using fallback. I know that earlier wcd93xx do not follow that concept,
but maybe we will fix them some point as well.

> +
> +  mode-switch:
> +    description: Flag the port as possible handle of altmode switching
> +    type: boolean
> +
> +  orientation-switch:
> +    description: Flag the port as possible handler of orientation switching
> +    type: boolean
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description:
> +      A port node to link the WCD939x Codec node to USB MUX subsystems for the
> +      purpose of handling altmode muxing and orientation switching to detecte and
> +      enable Audio Accessory Mode.
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    codec {
> +        compatible = "qcom,wcd9390-codec";
> +        reset-gpios = <&tlmm 32 0>;

Please define for the GPIO flag.

> +        #sound-dai-cells = <1>;
> +        qcom,tx-device = <&wcd939x_tx>;
> +        qcom,rx-device = <&wcd939x_rx>;
> +        qcom,micbias1-microvolt = <1800000>;
> +        qcom,micbias2-microvolt = <1800000>;
> +        qcom,micbias3-microvolt = <1800000>;
> +        qcom,micbias4-microvolt = <1800000>;
> +        qcom,hphl-jack-type-normally-closed;
> +        qcom,ground-jack-type-normally-closed;
> +        qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
> +        qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> +    };
> +
> +    /* ... */
> +
> +    soundwire@3210000 {
> +        #address-cells = <2>;
> +        #size-cells = <0>;
> +        reg = <0x03210000 0x2000>;
> +        wcd939x_rx: codec@0,4 {
> +            compatible = "sdw20217010e00";
> +            reg  = <0 4>;

Just one space before '='

> +            qcom,rx-port-mapping = <1 2 3 4 5 6>;
> +        };
> +    };
> +
> +    soundwire@3230000 {
> +        #address-cells = <2>;
> +        #size-cells = <0>;
> +        reg = <0x03230000 0x2000>;
> +        wcd938x_tx: codec@0,3 {
> +            compatible = "sdw20217010e00";
> +            reg  = <0 3>;

Ditto


Best regards,
Krzysztof
Neil Armstrong Nov. 28, 2023, 9:14 a.m. UTC | #2
On 28/11/2023 10:04, Krzysztof Kozlowski wrote:
> On 28/11/2023 09:59, Neil Armstrong wrote:
>> On 24/11/2023 09:33, Krzysztof Kozlowski wrote:
>>> On 23/11/2023 15:49, Neil Armstrong wrote:
>>>
>>>> +  Qualcomm WCD9390/WCD9395 Codec is a standalone Hi-Fi audio codec IC.
>>>> +  It has RX and TX Soundwire slave devices.
>>>> +  The WCD9390/WCD9395 IC has a functionally separate USB-C Mux subsystem
>>>> +  accessible over an I2C interface.
>>>> +  The Audio Headphone and Microphone data path between the Codec and the USB-C Mux
>>>> +  subsystems are external to the IC, thus requiring DT port-endpoint graph description
>>>> +  to handle USB-C altmode & orientation switching for Audio Accessory Mode.
>>>> +
>>>> +allOf:
>>>> +  - $ref: dai-common.yaml#
>>>> +  - $ref: qcom,wcd93xx-common.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - qcom,wcd9390-codec
>>>> +      - qcom,wcd9395-codec
>>>
>>> 9395 should be compatible with 9390, so please express it with a list
>>> using fallback. I know that earlier wcd93xx do not follow that concept,
>>> but maybe we will fix them some point as well.
>>
>> I don't get why this would be needed, yes their are compatible but still
>> two separate ICs with different internal capabilities.
>>
>> It the first time I get such request for new documentation
> 
> Maybe it is first time for you, but I ask about this all the time. What
> is important is whether the programming model or how the OS uses the
> device is the same.

I agree for new version of HW, anyway..

> 
> Here the device exposes its version in registers, so you can easily rely
> on the compatibility. That's also the case multiple times talked on the
> mailing lists.

... you're right here version can be determined at runtime.

But, since both are compatible, there's no primary part number, right?

so why use "qcom,wcd9395-codec", "qcom,wcd9390-codec"
when "qcom,wcd9390-codec", "qcom,wcd9395-codec" should
also be valid, so in this can why not use :
"qcom,wcd9390-codec", "qcom,wcd939x-codec"
or
"qcom,wcd9395-codec", "qcom,wcd939x-codec"

?

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Nov. 28, 2023, 9:17 a.m. UTC | #3
On 28/11/2023 10:14, neil.armstrong@linaro.org wrote:
>>
>> Here the device exposes its version in registers, so you can easily rely
>> on the compatibility. That's also the case multiple times talked on the
>> mailing lists.
> 
> ... you're right here version can be determined at runtime.
> 
> But, since both are compatible, there's no primary part number, right?
> 
> so why use "qcom,wcd9395-codec", "qcom,wcd9390-codec"

This one, please.

> when "qcom,wcd9390-codec", "qcom,wcd9395-codec" should
> also be valid, so in this can why not use :

Could be valid, sure, but we are humans and we treat higher number as
something newer or bigger, thus previous one feels more natural. There
are examples of this way, though.


> "qcom,wcd9390-codec", "qcom,wcd939x-codec"
> or
> "qcom,wcd9395-codec", "qcom,wcd939x-codec"

This not, because wildcards are not allowed in the compatibles. In the
past there were examples how a wildcard stopped being wild, so guideline
is: just don't use them.

Best regards,
Krzysztof
Neil Armstrong Nov. 28, 2023, 9:19 a.m. UTC | #4
On 28/11/2023 10:17, Krzysztof Kozlowski wrote:
> On 28/11/2023 10:14, neil.armstrong@linaro.org wrote:
>>>
>>> Here the device exposes its version in registers, so you can easily rely
>>> on the compatibility. That's also the case multiple times talked on the
>>> mailing lists.
>>
>> ... you're right here version can be determined at runtime.
>>
>> But, since both are compatible, there's no primary part number, right?
>>
>> so why use "qcom,wcd9395-codec", "qcom,wcd9390-codec"
> 
> This one, please.

Ok

> 
>> when "qcom,wcd9390-codec", "qcom,wcd9395-codec" should
>> also be valid, so in this can why not use :
> 
> Could be valid, sure, but we are humans and we treat higher number as
> something newer or bigger, thus previous one feels more natural. There
> are examples of this way, though.
> 
> 
>> "qcom,wcd9390-codec", "qcom,wcd939x-codec"
>> or
>> "qcom,wcd9395-codec", "qcom,wcd939x-codec"
> 
> This not, because wildcards are not allowed in the compatibles. In the
> past there were examples how a wildcard stopped being wild, so guideline
> is: just don't use them.
> 
> Best regards,
> Krzysztof
> 

Thanks,
Neil
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd939x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd939x-sdw.yaml
new file mode 100644
index 000000000000..7528c8b100a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd939x-sdw.yaml
@@ -0,0 +1,70 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/qcom,wcd939x-sdw.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SoundWire Slave devices on WCD9390/WCD9395
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: |
+  Qualcomm WCD9390/WCD9395 Codec is a standalone Hi-Fi audio codec IC.
+  It has RX and TX Soundwire slave devices. This bindings is for the
+  slave devices.
+
+properties:
+  compatible:
+    const: sdw20217010e00
+
+  reg:
+    maxItems: 1
+
+  qcom,tx-port-mapping:
+    description: |
+      Specifies static port mapping between slave and master tx ports.
+      In the order of slave port index.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 4
+    maxItems: 4
+
+  qcom,rx-port-mapping:
+    description: |
+      Specifies static port mapping between slave and master rx ports.
+      In the order of slave port index.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 6
+    maxItems: 6
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    soundwire@3210000 {
+        #address-cells = <2>;
+        #size-cells = <0>;
+        reg = <0x03210000 0x2000>;
+        wcd938x_rx: codec@0,4 {
+            compatible = "sdw20217010e00";
+            reg  = <0 4>;
+            qcom,rx-port-mapping = <1 2 3 4 5 6>;
+        };
+    };
+
+    soundwire@3230000 {
+        #address-cells = <2>;
+        #size-cells = <0>;
+        reg = <0x03230000 0x2000>;
+        wcd938x_tx: codec@0,3 {
+            compatible = "sdw20217010e00";
+            reg  = <0 3>;
+            qcom,tx-port-mapping = <2 3 4 5>;
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd939x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd939x.yaml
new file mode 100644
index 000000000000..39c1de6961ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd939x.yaml
@@ -0,0 +1,93 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/qcom,wcd939x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm WCD9380/WCD9385 Audio Codec
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: |
+  Qualcomm WCD9390/WCD9395 Codec is a standalone Hi-Fi audio codec IC.
+  It has RX and TX Soundwire slave devices.
+  The WCD9390/WCD9395 IC has a functionally separate USB-C Mux subsystem
+  accessible over an I2C interface.
+  The Audio Headphone and Microphone data path between the Codec and the USB-C Mux
+  subsystems are external to the IC, thus requiring DT port-endpoint graph description
+  to handle USB-C altmode & orientation switching for Audio Accessory Mode.
+
+allOf:
+  - $ref: dai-common.yaml#
+  - $ref: qcom,wcd93xx-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - qcom,wcd9390-codec
+      - qcom,wcd9395-codec
+
+  mode-switch:
+    description: Flag the port as possible handle of altmode switching
+    type: boolean
+
+  orientation-switch:
+    description: Flag the port as possible handler of orientation switching
+    type: boolean
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description:
+      A port node to link the WCD939x Codec node to USB MUX subsystems for the
+      purpose of handling altmode muxing and orientation switching to detecte and
+      enable Audio Accessory Mode.
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    codec {
+        compatible = "qcom,wcd9390-codec";
+        reset-gpios = <&tlmm 32 0>;
+        #sound-dai-cells = <1>;
+        qcom,tx-device = <&wcd939x_tx>;
+        qcom,rx-device = <&wcd939x_rx>;
+        qcom,micbias1-microvolt = <1800000>;
+        qcom,micbias2-microvolt = <1800000>;
+        qcom,micbias3-microvolt = <1800000>;
+        qcom,micbias4-microvolt = <1800000>;
+        qcom,hphl-jack-type-normally-closed;
+        qcom,ground-jack-type-normally-closed;
+        qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
+        qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
+    };
+
+    /* ... */
+
+    soundwire@3210000 {
+        #address-cells = <2>;
+        #size-cells = <0>;
+        reg = <0x03210000 0x2000>;
+        wcd939x_rx: codec@0,4 {
+            compatible = "sdw20217010e00";
+            reg  = <0 4>;
+            qcom,rx-port-mapping = <1 2 3 4 5 6>;
+        };
+    };
+
+    soundwire@3230000 {
+        #address-cells = <2>;
+        #size-cells = <0>;
+        reg = <0x03230000 0x2000>;
+        wcd938x_tx: codec@0,3 {
+            compatible = "sdw20217010e00";
+            reg  = <0 3>;
+            qcom,tx-port-mapping = <2 3 4 5>;
+        };
+    };
+
+...