Re: [PATCH V9 1/3] dt-bindings: soundwire: qcom: Increase max data ports to 17

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Globally increasing maxItems to 17 for port properties weakens schema validation for older SoundWire controllers that only support 16 ports.

Pre-existing issues:
- [High] Port allocation logic fails to properly check if the highest port is busy, leading to silent collisions where the maximum port index is allocated repeatedly when all lower ports are exhausted.
--

commit 4f104dadbb8cc83ffee25439c24bc3ee896cb832
Author: Sibi Sankar <sibi.sankar@oss.qualcomm.com>

dt-bindings: soundwire: qcom: Increase max data ports to 17

This patch bumps the maxItems from 16 to 17 for all qcom,ports-* properties.
This accommodates SoundWire controllers v3.1.0 which feature 17 data ports.

> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> index 9447a2f371b56..8e6973fa229c9 100644
> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> @@ -90,7 +90,7 @@ properties:
>        or applicable for the respective data port.
>        More info in MIPI Alliance SoundWire 1.0 Specifications.
>      minItems: 3
> -    maxItems: 16
> +    maxItems: 17

[Severity: Medium]
By globally increasing maxItems to 17, does this weaken schema validation
for older SoundWire controllers that physically only support 16 ports?

A device tree for older hardware (e.g., v1.3.0 to v2.2.0) could now incorrectly
specify 17 ports and pass dt_binding_check.

Should this use conditional allOf / if-then blocks based on the compatible
string to enforce generation-specific bounds?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a pre-existing
logic error in the dynamic port allocation that these bounds protect.

Looking at qcom_swrm_stream_alloc_ports():

drivers/soundwire/qcom.c:qcom_swrm_stream_alloc_ports() {
    ...
    pn = find_first_zero_bit(port_mask, maxport);

    if (pn > maxport) {
        dev_err(ctrl->dev, "All ports busy\n");
        return -EBUSY;
    }
    set_bit(pn, port_mask);
    ...
}

Because find_first_zero_bit() searches the range [0, maxport-1], it returns
maxport if no zero bits are found. If ports 1 to maxport-1 are all busy, the
check "if (pn > maxport)" evaluates to false.

Does this allow the code to blindly assign maxport and set its bit without
verifying if it was actually free?

It seems on subsequent allocations, the search will again find no zero bits and
repeatedly assign maxport, causing silent resource collisions where multiple
audio streams share the same hardware port.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523085511.2532669-1-sibi.sankar@oss.qualcomm.com?part=1