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

[Prasad Kumpatla]

On 6/11/2026 3:04 PM, Krzysztof Kozlowski wrote:
> 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.

Hi Krzysztof,

Thanks for reviewing the patch and for the feedback.

Ack, Will add more HW details in next version.

> > 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.

Ack, Will remove.

> > @@ -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.
Ack, Will Update.
> > +  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?

Ack, Thanks for the reference link, will cross check and update the 
bindings.

No, there is o internal review done for this patch due to timelines.

> > +
> > +  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?


Agree, Will remove this and come up standard interrupt bindings in next 
version.

> > +    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
Ack.
> > +    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.


Ack,I'll move them into the driver as a register table,

making them easier to maintain and avoiding opaque DT data.

> > +      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.

Thanks for pointing this out. I'll align the schema with the existing 
WSA88xx

bindings and replace additionalProperties: false with 
unevaluatedProperties: false

in the next revision.

Thanks,

Prasad

> Best regards,
> Krzysztof