mbox series

[v2,0/8] ASoC: codecs: wcd937x: add wcd937x audio codec support

Message ID 20240416063600.309747-1-quic_mohs@quicinc.com
Headers show
Series ASoC: codecs: wcd937x: add wcd937x audio codec support | expand

Message

Mohammad Rafi Shaik April 16, 2024, 6:35 a.m. UTC
This patchset adds support for Qualcomm WCD937X codec.

Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC
connected over SoundWire. This device has two SoundWire devices, RX and
TX respectively supporting 3 x ADCs, ClassH, Ear, Aux PA, 2xHPH,
6 DMICs and MBHC.

For codec driver to be functional it would need both tx and rx Soundwire devices
to be up and this is taken care by using device component framework and device-links
are used to ensure proper pm dependencies. Ex tx does not enter suspend
before rx or codec is suspended.

This patchset along with other SoundWire patches on the list
have been tested on QCM6490 IDP device.

Changes since v8:
 - Split the patch per driver for easier review as suggested by Krzysztof
 - Used devm_gpiod_get api to get reset gpio as suggested by Krzysztof

Prasad Kumpatla (8):
  ASoC: dt-bindings: wcd937x: add bindings for wcd937x
  ASoC: codecs: wcd937x: add wcd937x codec driver
  ASoC: dt-bindings: wcd937x-sdw: add bindings for wcd937x-sdw
  ASoC: codecs: wcd937x-sdw: add SoundWire driver
  ASoC: codecs: wcd937x: add basic controls
  ASoC: codecs: wcd937x: add playback dapm widgets
  ASoC: codecs: wcd937x: add capture dapm widgets
  ASoC: codecs: wcd937x: add audio routing and Kconfig

 .../bindings/sound/qcom,wcd937x-sdw.yaml      |   71 +
 .../bindings/sound/qcom,wcd937x.yaml          |  119 +
 sound/soc/codecs/Kconfig                      |   20 +
 sound/soc/codecs/Makefile                     |    7 +
 sound/soc/codecs/wcd937x-sdw.c                | 1148 +++++++
 sound/soc/codecs/wcd937x.c                    | 3036 +++++++++++++++++
 sound/soc/codecs/wcd937x.h                    |  655 ++++
 7 files changed, 5056 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
 create mode 100644 sound/soc/codecs/wcd937x-sdw.c
 create mode 100644 sound/soc/codecs/wcd937x.c
 create mode 100644 sound/soc/codecs/wcd937x.h


base-commit: 6bd343537461b57f3efe5dfc5fc193a232dfef1e

Comments

Dmitry Baryshkov April 16, 2024, 12:40 p.m. UTC | #1
On Tue, 16 Apr 2024 at 09:36, Mohammad Rafi Shaik <quic_mohs@quicinc.com> wrote:
>
> This patchset adds support for Qualcomm WCD937X codec.
>
> Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC
> connected over SoundWire. This device has two SoundWire devices, RX and
> TX respectively supporting 3 x ADCs, ClassH, Ear, Aux PA, 2xHPH,
> 6 DMICs and MBHC.
>
> For codec driver to be functional it would need both tx and rx Soundwire devices
> to be up and this is taken care by using device component framework and device-links
> are used to ensure proper pm dependencies. Ex tx does not enter suspend
> before rx or codec is suspended.
>
> This patchset along with other SoundWire patches on the list
> have been tested on QCM6490 IDP device.
>
> Changes since v8:

I hope it's a typo here since the series is v2, not v9

>  - Split the patch per driver for easier review as suggested by Krzysztof
>  - Used devm_gpiod_get api to get reset gpio as suggested by Krzysztof
>
> Prasad Kumpatla (8):
>   ASoC: dt-bindings: wcd937x: add bindings for wcd937x
>   ASoC: codecs: wcd937x: add wcd937x codec driver
>   ASoC: dt-bindings: wcd937x-sdw: add bindings for wcd937x-sdw
>   ASoC: codecs: wcd937x-sdw: add SoundWire driver
>   ASoC: codecs: wcd937x: add basic controls
>   ASoC: codecs: wcd937x: add playback dapm widgets
>   ASoC: codecs: wcd937x: add capture dapm widgets
>   ASoC: codecs: wcd937x: add audio routing and Kconfig
>
>  .../bindings/sound/qcom,wcd937x-sdw.yaml      |   71 +
>  .../bindings/sound/qcom,wcd937x.yaml          |  119 +
>  sound/soc/codecs/Kconfig                      |   20 +
>  sound/soc/codecs/Makefile                     |    7 +
>  sound/soc/codecs/wcd937x-sdw.c                | 1148 +++++++
>  sound/soc/codecs/wcd937x.c                    | 3036 +++++++++++++++++
>  sound/soc/codecs/wcd937x.h                    |  655 ++++
>  7 files changed, 5056 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>  create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
>  create mode 100644 sound/soc/codecs/wcd937x-sdw.c
>  create mode 100644 sound/soc/codecs/wcd937x.c
>  create mode 100644 sound/soc/codecs/wcd937x.h
>
>
> base-commit: 6bd343537461b57f3efe5dfc5fc193a232dfef1e
> --
> 2.25.1
>
>
Krzysztof Kozlowski April 17, 2024, 3:56 p.m. UTC | #2
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 devices RX and
> TX respectively.
> This binding is for those slave devices on WCD9370/WCD9375.
> 
> 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-sdw.yaml      | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
> new file mode 100644
> index 000000000000..2b7358e266ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SoundWire Slave devices on WCD9370
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  Qualcomm WCD9370 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: sdw20217010a00
> +
> +  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.

Use inclusive terminology. Describe what is here - what is the index?
What is the value?

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 4
> +    maxItems: 4

Add constraints on values. You have maximum 15 TX ports, don't you?

> +
> +  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: 5
> +    maxItems: 5
> +
> +required:
> +  - compatible
> +  - reg
> +  - qcom,port-mapping

Test your binding. There is no need to engage reviewers for reviewing
simple mistakes which *tools* can point. Respect reviewers time and use
the tools first.

You need oneOf: with required for TX and RX... or just unify the
properties. Why do you need two?


> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soundwire@3210000 {
Drop unit address.

> +        #address-cells = <2>;
> +        #size-cells = <0>;
> +        reg = <0x03210000 0x2000>;

Drop, not relevant and not placed correctly (see DTS coding style).

> +        wcd937x_rx: codec@0,4 {

Drop label, not used.

> +            compatible = "sdw20217010a00";
> +            reg  = <0 4>;
> +            qcom,rx-port-mapping = <1 2 3 4 5>;
> +        };
> +    };
> +
> +    soundwire@3230000 {

Drop this example, it's almost identical.

Best regards,
Krzysztof