Re: [PATCH V1] dt-bindings: phy: qcom,sc8280xp-qmp-ufs-phy: Add QMP UFS PHY compatible for Maili

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The new compatible strings `qcom,maili-qmp-ufs-phy` and `qcom,hawi-qmp-ufs-phy` are omitted from the `allOf` conditional blocks that enforce strict constraints for clocks.
- [Medium] The fallback compatible `qcom,hawi-qmp-ufs-phy` is not defined as a standalone base compatible in the schema.
--

--- Patch [1]: [PATCH V1] dt-bindings: phy: qcom,sc8280xp-qmp-ufs-phy: Add QMP UFS PHY compatible for Maili ---
commit c983ddf99ae7f556a6b7a2870c88327054b94d8c
Author: Nitin Rawat <[email protected]>

dt-bindings: phy: qcom,sc8280xp-qmp-ufs-phy: Add QMP UFS PHY compatible for Maili

Document the QMP UFS PHY compatible for Qualcomm Maili to support
physical layer functionality for UFS found on the SoC. Use fallback to
indicate the compatibility of the QMP UFS PHY on the Maili with that on
the Hawi.

> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> index b2c5c9a375a35..f74434f51653c 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> @@ -16,6 +16,10 @@ description:
>  properties:
>    compatible:
>      oneOf:
> +      - items:
> +          - enum:
> +              - qcom,maili-qmp-ufs-phy
> +          - const: qcom,hawi-qmp-ufs-phy

[Severity: Medium]
If qcom,hawi-qmp-ufs-phy is used as a fallback here, shouldn't it also
be added to the standalone enum list further down in this file at line 42
under properties:compatible:oneOf:enum?

Without it being defined as a standalone compatible string, a device tree
node for the base Hawi hardware using just
compatible = "qcom,hawi-qmp-ufs-phy" will fail schema validation when running
dt_binding_check.


[Severity: Medium]
Do these new compatibles need to be added to the allOf conditional block
that enforces strict clock constraints at line 88?

Currently, Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
has an allOf:if block starting at line 88 that enforces a strict 3-clock
limit (ref, ref_aux, qref) for most modern PHYs.

Since they are missing from that enum list, device trees using these new
compatible strings will silently bypass the strict clock constraints and
fall back to the loose global defaults. This regression allows incomplete or
invalid clock configurations to pass dt_binding_check validation.

>        - items:
>            - enum:
>                - qcom,qcs8300-qmp-ufs-phy

-- 
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