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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The schema incorrectly uses `additionalProperties: false` while including a `$ref` to `dai-common.yaml#`, which will erroneously reject valid standard DAI properties.
- [Medium] The binding uses a custom GPIO property (`interrupt-gpios`) to model a hardware interrupt line instead of the standard `interrupts` property.
- [Medium] The `qcom,wsa885x-init-table` property inappropriately encodes raw hardware register scripts into the Device Tree.
- [Low] Wildcard ('x') used in the compatible string instead of specific hardware part numbers.
--

commit 535bce1da75b7ff8bde3534f15f04d5dbbcc99a2
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>

dt-bindings: sound: add qcom,wsa885x-i2c

This commit documents the Qualcomm WSA885X I2C smart amplifier binding. It 
describes the required supplies, powerdown and interrupt GPIOs, battery 
configuration, and an initialization table for programming the device.

> 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 0000000000000..1069f470dea5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
> @@ -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

[ ... ]

> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,wsa885x-i2c

[Severity: Low]
Is it standard to use a wildcard (x) in the compatible string here? 

Devicetree typically requires specific hardware part numbers, such as 
qcom,wsa8850-i2c, to ensure future variant quirks or different tuning 
requirements can be handled properly without ambiguity.

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

[Severity: Medium]
Does this hardware interrupt line need to be modeled as a raw GPIO property 
rather than using the standard interrupts and interrupt-parent properties? 

Modeling it as a standard interrupt allows proper subsystem integration and 
routing, whereas interrupt-gpios bypasses the standard IRQ hierarchy.

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

[Severity: Medium]
Does this property encode raw hardware register scripts directly into the 
devicetree? 

The devicetree is generally meant to describe static hardware topology rather 
than act as a repository for magic register initialization sequences. 
Typically, this sort of initialization logic resides within the driver code 
itself, keyed off specific compatible strings.

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#sound-dai-cells'
> +  - powerdown-gpios
> +  - interrupt-gpios
> +  - vdd-1p8-supply
> +  - vdd-io-supply
> +
> +additionalProperties: false

[Severity: Medium]
Since this binding includes a reference to dai-common.yaml# in the allOf 
block above, will using additionalProperties: false cause standard DAI 
properties to be erroneously rejected? 

The additionalProperties keyword only accounts for properties explicitly 
defined in the local schema. The standard approach in this scenario is to 
use unevaluatedProperties: false so that properties from the referenced 
schema are correctly validated.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610155708.151067-1-prasad.kumpatla@oss.qualcomm.com?part=1