Re: [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `clock-names` definition for `qcom,x1e80100-qmp-usb3-dp-phy` implicitly makes the 5th clock mandatory, breaking validation for existing device trees.
- [Medium] The `p2rr2p_pipe` clock is artificially restricted to the X1E SoC, preventing other USB4-capable SoCs in the same binding from modeling their hardware correctly.
--

--- Patch [1]: [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4 ---
commit 86283e120a292e73e741d8fc0d233382e691770a
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4

This commit extends the X1E USB3/DP PHY bindings to support USB4 mode by
allowing an additional clock (P2RR2P). It adds conditionals to handle the
differences in clock requirements between earlier generations and X1E.

> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> index 4eff92343ce4f..18930eb96bdfe 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml

[ ... ]

> @@ -187,21 +182,59 @@ allOf:
>    - if:
>        properties:
>          compatible:
> -          enum:
> -            - qcom,sc7180-qmp-usb3-dp-phy
> -            - qcom,sdm845-qmp-usb3-dp-phy
> +          contains:
> +            enum:
> +              - qcom,x1e80100-qmp-usb3-dp-phy
>      then:
>        properties:
>          clocks:
>            maxItems: 5
>          clock-names:
> +          items:
> +            - const: aux
> +            - const: ref
> +            - const: com_aux
> +            - const: usb3_pipe
> +            - const: p2rr2p_pipe

[Severity: High]
Will this break device tree validation for existing x1e80100 device trees
that only provide 4 clocks? Since minItems is not explicitly set in this
local subschema, dt-schema will automatically set minItems to 5 to match
the length of the items array. Should minItems: 4 be added to this block to
preserve backward compatibility?

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sc7180-qmp-usb3-dp-phy
> +              - qcom,sdm845-qmp-usb3-dp-phy
> +    then:
> +      properties:
> +        clocks:
>            maxItems: 5
> -    else:
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: ref
> +            - const: com_aux
> +            - const: usb3_pipe
> +            - const: cfg_ahb
> +
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - qcom,x1e80100-qmp-usb3-dp-phy
> +                - qcom,sc7180-qmp-usb3-dp-phy
> +                - qcom,sdm845-qmp-usb3-dp-phy

[Severity: Medium]
Does this artificial restriction to maxItems: 4 for all other compatibles
prevent other USB4-capable SoCs (like sc8280xp) from correctly modeling
their hardware?

If they share the same USB43DP hardware capabilities and available clock
lines, they will fail validation if a developer attempts to add the
p2rr2p_pipe clock to them in the future.

Could this be structured to allow the 5th clock on other identical hardware
variants?

> +    then:
>        properties:
>          clocks:
>            maxItems: 4
>          clock-names:
> -          maxItems: 4
> +          items:
> +            - const: aux
> +            - const: ref
> +            - const: com_aux
> +            - const: usb3_pipe

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-topic-usb4phy-v3-0-ccf0f7ea98b5@oss.qualcomm.com?part=1