Re: [PATCH v2 01/11] dt-bindings: sound: qcom,qaif-cpu: Add binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The property `qcom,qaif-aif-bits-per-lane` is fundamentally misnamed relative to its description and usage, creating a dangerous contradiction that will lead to incorrect hardware configurations.
- [Medium] The `qcom,qaif-aif-lane-config` matrix schema fails to enforce the exact length of each row, allowing malformed configurations to silently pass validation.
- [Low] The `compatible` property schema forbids standard specific-to-generic fallback chains.
--

commit 766077d779dbf4ceb1f7492bb1a02c81dfc5f0d3
Author: Harendra Gautam <harendra.gautam@oss.qualcomm.com>

dt-bindings: sound: qcom,qaif-cpu: Add binding

Add a dt-bindings header and devicetree binding for the Qualcomm Audio
Interface (QAIF) CPU DAI controller. This allows platform devicetree
files to describe the controller's MMIO region, IOMMU stream, clocks,
interrupts, and per-interface AIF configuration.

Depends-on: [PATCH 0/4] clk: qcom: Add Audio Core clock controller support on Qualcomm Shikra SoC
  https://lore.kernel.org/linux-clk/20260605-shikra-audiocorecc-v1-0-7ee6b5f2d928@oss.qualcomm.com/

> diff --git a/Documentation/devicetree/bindings/sound/qcom,qaif-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,qaif-cpu.yaml
> new file mode 100644
> index 0000000000000..672f40df877be
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,qaif-cpu.yaml
> @@ -0,0 +1,358 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/qcom,qaif-cpu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - qcom,qaif-cpu
> +      - qcom,shikra-qaif-cpu

[Severity: Low]
Does this strict enum restrict the compatible property to a single string,
preventing standard specific-to-generic fallback chains? If a device tree
attempts to use a fallback like "qcom,shikra-qaif-cpu", "qcom,qaif-cpu",
it looks like dt_binding_check validation would fail.

[ ... ]

> +      qcom,qaif-aif-lane-config:
> +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +        description:
> +          Lane configuration matrix. Each row is a pair <enable direction>
> +          for one lane starting from lane 0, up to 8 lanes.
> +          enable: 0 = disabled, 1 = enabled (QAIF_AIF_LANE_ENABLE).
> +          direction: QAIF_AIF_LANE_DIR_TX (0) for TX (playback),
> +                     QAIF_AIF_LANE_DIR_RX (1) for RX (capture).
> +        maxItems: 8
> +        items:
> +          items:
> +            - description: Lane enable (0 = disabled, 1 = enabled)
> +              enum: [0, 1]
> +            - description: Lane direction (0 = TX/speaker, 1 = RX/mic)
> +              enum: [0, 1]

[Severity: Medium]
Should this inner items list include minItems: 2 and maxItems: 2?

Since JSON Schema Draft 7 doesn't implicitly restrict array tuple lengths,
omitting the bounds allows extra elements per row to silently pass validation.

If malformed arrays are provided, it looks like the driver's flat array
parsing would misalign and misinterpret the values.

> +      qcom,qaif-aif-full-cycle-en:
> +        type: boolean
> +        description: Enable full-cycle sync (effective in sync master mode).
> +      qcom,qaif-aif-bits-per-lane:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Number of slots per lane. Maps to hardware register
> +          BITS_PER_LANE as (slot_width * bits_per_lane - 1).
> +          For example, 2 slots with a 32-bit slot width gives
> +          63 (= 32 * 2 - 1) in the register, representing 64
> +          bits per lane per frame.

[Severity: High]
Is this property fundamentally misnamed? The description says it represents
the number of slots per lane, and the driver code also appears to treat
it as slots to calculate the final register value.

If a device tree author follows the property name qcom,qaif-aif-bits-per-lane
and provides the actual number of bits (like 64), wouldn't this cause a
register overflow and clock miscalculation when the driver multiplies
it by slot_width?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701105757.2779738-1-harendra.gautam@oss.qualcomm.com?part=1