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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `allOf` conditionals for `x1e80100` and `sc7180`/`sdm845` unintentionally make the 5th clock mandatory, breaking schema backward compatibility for device trees that only provide 4 clocks.
--

--- Patch [1]: [PATCH v4 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4 ---
commit 16b464b2f16cdfe9906295a8ca043c6c92f8b970
Author: Konrad Dybcio <[email protected]>

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

This commit adds a new USB4 clock to the X1E QMP combo PHY binding
to enable USB4 mode. It restructures the schema's allOf conditionals to
apply specific clock lists based on the compatible string.

> 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 8adfc78108fef..94a4fbff428f4 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml

[ ... ]

> @@ -189,21 +184,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: Medium]
Does this inadvertently make the 5th clock mandatory for x1e80100?

Looking at the allOf conditional for qcom,x1e80100-qmp-usb3-dp-phy in
Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml,
since this items array defines 5 elements, dt-schema automatically injects
a minimum item count of 5 locally for this conditional block.

This overrides the global minimum item count of 4, which would cause schema
validation to fail for existing older device trees that only provide 4 clocks.

Should an explicit minimum item count of 4 be added here to maintain 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

[Severity: Medium]
Similarly, does this break backward compatibility for sc7180 and sdm845?

Looking at the allOf conditional for qcom,sc7180-qmp-usb3-dp-phy, without
an explicit minimum item count of 4 in this block, older device trees that
omit cfg_ahb will fail validation. The dt-schema tool will enforce exactly
5 items based on the array length.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy