Re: [PATCH v3 2/4] dt-bindings: phy: qcom,qcs615-qmp-usb3-dp-phy: Add support for Shikra

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] QCS615 is an invalid fallback for Shikra due to differing hardware reset requirements and removed DisplayPort functionality.
- [Medium] Missing `allOf` conditional blocks to enforce correct reset counts for each SoC variant.
--

commit fb0664de6e00aa81793b7c92908a732069771e68
Author: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>

dt-bindings: phy: qcom,qcs615-qmp-usb3-dp-phy: Add support for Shikra

This commit extends the QCS615 QMP USB3/DP PHY bindings to add support for
the Shikra SoC. It introduces a new compatible string with QCS615 as a
fallback, and increases the maximum number of resets to accommodate Shikra.

> Declare the USB QMP Phy present on Shikra SoC. On this SoC, although
> there are DP registers, the required internal pins have been removed
> and hence DP wouldn't be operational.
> 
> Shikra needs 3 resets (dp/ phy/ phy_phy) to be asserted and de-asserted
> as per hardware recommendation. Extend the bindings to accommodate the
> additional reset.
> 
> Use QCS615 as a fallback since the software interface is compatible with
> Shikra.

[Severity: High]
Is QCS615 a valid fallback for the Shikra SoC here?

The commit message notes that Shikra requires an additional third reset
(phy) per hardware recommendations, and that DisplayPort functionality is
non-operational. If an older kernel matches the Shikra device using the
QCS615 fallback driver, it will fail to assert the required third reset and
will erroneously attempt to initialize the physically disconnected DP
functionality.

Could this lead to incomplete hardware initialization and potential
subsystem timeouts?

> diff --git a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-usb3dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-usb3dp-phy.yaml
> index efb465c71c1b5..65ae4c6e09db0 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-usb3dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-usb3dp-phy.yaml
[ ... ]
> @@ -33,12 +37,15 @@ properties:
>        - const: pipe
>  
>    resets:
> -    maxItems: 2
> +    minItems: 2
> +    maxItems: 3
>  
>    reset-names:
> +    minItems: 2
>      items:
>        - const: phy_phy
>        - const: dp_phy
> +      - const: phy

[Severity: Medium]
Will this allow invalid device tree configurations to silently pass schema
validation?

By relaxing the global constraints to allow up to 3 resets, a QCS615 node
could incorrectly specify 3 resets, or a Shikra node could incorrectly
specify only 2.

Should we add an allOf section with if-then conditional blocks to enforce
the exact required counts for each SoC variant (exactly 2 for QCS615, and
exactly 3 for Shikra)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526184401.3959717-1-pratham.pratap@oss.qualcomm.com?part=2