Re: [PATCH v1 1/2] dt-bindings: sound: add qcom,wsa885x-i2c

[Krzysztof Kozlowski]

On Wed, Jun 10, 2026 at 09:27:07PM +0530, Prasad Kumpatla wrote:
> Document the Qualcomm WSA885X I2C smart amplifier binding.
> 
> Describe the required supplies, powerdown and interrupt GPIOs, the
> optional battery configuration, and the optional init-table property
> used to program the device during codec initialization.
> 
> This matches the driver programming model and documents the DT data

Binding matches hardware, not driver. Please describe the hardware.

> needed to use the codec on platforms with Audio IF playback.
> 
> Signed-off-by: Prasad Kumpatla <prasad.kumpatla@xxxxxxxxxxxxxxxx>
> ---
>  .../bindings/sound/qcom,wsa885x-i2c.yaml      | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml b/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
> new file mode 100644
> index 000000000..1069f470d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml

There is no I2C in device name.

> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/qcom,wsa885x-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm WSA885x I2C smart speaker amplifier
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxxxxxxxx>
> +  - Prasad Kumpatla <prasad.kumpatla@xxxxxxxxxxxxxxxx>
> +
> +description: |

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

> +  WSA885x is a Qualcomm Aqstic smart speaker amplifier with an I2C control
> +  interface and a digital audio interface exposed through ASoC DAI callbacks.
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,wsa885x-i2c

Same here

Also, incorrect usage of wildcard. Look at other bindings how this is
written, so you will not repeat the same comments:
https://lore.kernel.org/all/20250522-rb2_audio_v3-v3-3-9eeb08cab9dc@xxxxxxxxxx/

Read writing bindings before posting next version.

I also cannot find traces of internal review of this. Did it happen? Did
you receive toolset comments?

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#sound-dai-cells':
> +    const: 0
> +
> +  powerdown-gpios:
> +    description: GPIO controlling the SD_N powerdown pin.
> +    maxItems: 1
> +
> +  interrupt-gpios:

No, interrupts are never written as GPIOs.

Where is this binding coming from?

> +    description: GPIO used for the codec interrupt output.
> +    maxItems: 1
> +
> +  vdd-1p8-supply: true
> +
> +  vdd-io-supply: true
> +
> +  qcom,battery-config:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Speaker battery configuration, 1 for 1S and 2 for 2S.

Use string

> +    default: 1
> +    enum: [1, 2]
> +
> +  qcom,wsa885x-init-table:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    maxItems: 256
> +    description: |
> +      Sequence of register/value pairs applied during codec hardware

No, we don't store register values usually.

> +      initialization. Entries are encoded as alternating register address and
> +      register value cells. The number of entries must be even (register/value
> +      pairs); maxItems is 256 (128 pairs).
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#sound-dai-cells'
> +  - powerdown-gpios
> +  - interrupt-gpios
> +  - vdd-1p8-supply
> +  - vdd-io-supply
> +
> +additionalProperties: false

unevaluated instead. Again, OPEN other existing bindings. Why doing
something completely different? Is there any WSA88xx binding with
additionalProperties? No.

Best regards,
Krzysztof