Re: [PATCH v13 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 2 potential issue(s) to consider:
- [Medium] The newly added standalone `qcom,x1e80100-csi2-phy` binding claims hardware resources and changes the OF graph topology without matching updates to the existing `qcom,x1e80100-camss.yaml` schema.
- [Medium] The `patternProperties` for `port@0` allows arbitrary endpoint nodes, bypassing the split-mode data-lane constraints.
--

--- Patch [1]: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema ---
commit 21e0c2eac81f8a0822a401dad73b1be1b2b38b5e
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

This commit adds a new base schema for the MIPI CSI2 PHYs on Qualcomm
SoCs, supporting DPHY modes (with CPHY left for future work). It includes
support for a split-mode where two sensors can be connected to one PHY.

> 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..880fe602945cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> @@ -0,0 +1,195 @@

[ ... ]

> +properties:
> +  compatible:
> +    const: qcom,x1e80100-csi2-phy
> +
> +  reg:
> +    maxItems: 1

[Severity: Medium]
Does this new reg definition overlap with the base addresses already
claimed by qcom,x1e80100-camss.yaml?

If a device tree implements this new binding alongside the existing CAMSS
binding, it seems it will produce dtc warnings for overlapping unit
addresses. Furthermore, drivers might fail to probe due to -EBUSY when
claiming overlapping memory regions like 0x0ace4000.

[ ... ]

> +  vdda-1p2-supply:
> +    description: Phandle to 1.2V regulator supply to a PHY.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports

[Severity: Medium]
Will these port definitions change the DT topology by placing the PHY as
an intermediate node?

The existing CAMSS binding ports currently assume a direct sensor
connection. Do we need matching updates in qcom,x1e80100-camss.yaml to
accommodate this new standalone PHY binding?

> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description:
> +          Sensor input. Always present. A single sensor is described by a
> +          single endpoint with one to four data lanes. DPHY split mode,
> +          where two independent sensors share the same PHY, is described
> +          by two endpoints; endpoint@0 with exactly two data-lanes and
> +          endpoint@1 with exactly one data-lane.
> +        unevaluatedProperties: false
> +
> +        patternProperties:
> +          "^endpoint(@[0-9a-f]+)?$":
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +            required:
> +              - data-lanes
> +              - remote-endpoint
> +
> +        allOf:
> +          - if:
> +              required:
> +                - endpoint@1

[Severity: Medium]
Does this constraint logic leave a loophole for invalid split-mode
configurations?

Because the if-block only evaluates to true when endpoint@1 is present,
a device tree configuring endpoint@0 and endpoint@2 would bypass the
strict data-lane limits.

Both endpoints would fall back to the generic rule above, allowing up to
4 lanes each, which silently circumvents the split-mode hardware
constraints (which only allow 2 and 1 lanes respectively). Should the
patternProperties be restricted to exactly endpoint@0 and endpoint@1, or
the if-condition broadened?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-x1e-csi2-phy-v13-0-160c31958863@linaro.org?part=1