Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Using the literal `endpoint:` property name inside ports bypasses constraint validation for endpoints with unit addresses.
- [Medium] The `data-lanes` property on `port@1` is overly restricted to `maxItems: 1`, which will break multi-lane DPHY split-mode configurations.
- [Low] The example uses a non-generic device tree node name `csiphy@...` instead of the standard `phy@...`.
- [Low] Mismatch between the `vdda-0p9-supply` property name and the actual 0.8V hardware supply used on the x1e80100 platform.
--

commit 10936c65e44aeea784cc13676d51700996cf6559
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema

Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
PHY devices.

> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> new file mode 100644
> index 0000000000000..270375f949880
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
[ ... ]
> +  power-domain-names:
> +    items:
> +      - const: mmcx
> +      - const: mx
> +
> +  vdda-0p9-supply:
> +    description: Phandle to a 0.9V regulator supply to a PHY.

[Severity: Low]
Does this property name mismatch the actual hardware supply voltage for the
x1e80100 platform?

The example later in this schema uses <&vreg_l2c_0p8>, which indicates an
0.8V rail. Should this be updated to reflect the 0.8V voltage?

> +
> +  vdda-1p2-supply:
> +    description: Phandle to 1.2V regulator supply to a PHY.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description: Sensor input. Always present.
> +        unevaluatedProperties: false
> +
> +        properties:
> +          endpoint:

[Severity: Medium]
Will using the literal endpoint: key bypass constraint validation if an
endpoint with a unit address (e.g., endpoint@0) is used?

Because the port references port-base, such an endpoint will pass the
unevaluatedProperties: false check, but its specific constraints wouldn't
be validated. Could this use patternProperties: "^endpoint(@[0-9a-f]+)?$":
instead?

> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
[ ... ]
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description:
> +          Second sensor input. When present, indicates DPHY split mode.
> +        unevaluatedProperties: false
> +
> +        properties:
> +          endpoint:

[Severity: Medium]
As with port@0, should this use patternProperties: "^endpoint(@[0-9a-f]+)?$":
to ensure endpoints with unit addresses are properly validated?

> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                maxItems: 1

[Severity: Medium]
Is maxItems: 1 too restrictive for multi-lane DPHY split-mode configurations?

Split-mode typically divides a 4-lane PHY into two independent 2-lane
interfaces (e.g., data-lanes = <2 3>). This configuration would fail
dt_binding_check validation with a single item limit.

> +              clock-lanes:
> +                maxItems: 1
[ ... ]
> +      port@2:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description: Output to CAMSS controller.
> +        unevaluatedProperties: false
> +
> +        properties:
> +          endpoint:

[Severity: Medium]
As above, should this use patternProperties: "^endpoint(@[0-9a-f]+)?$":?

> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            unevaluatedProperties: false
[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
> +    #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
> +    #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> +    csiphy4: csiphy@ace4000 {

[Severity: Low]
Does this node name violate the standard naming convention for PHY providers?

The core phy-provider.yaml schema expects node names to match the generic
pattern ^phy(@.*)?$. The current name csiphy@ace4000 will likely trigger a
dt_binding_check warning.

> +        compatible = "qcom,x1e80100-csi2-phy";
> +        reg = <0x0ace4000 0x2000>;
> +        #phy-cells = <1>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523-x1e-csi2-phy-v8-0-a85668459521@linaro.org?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy