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

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema incorrectly permits CPHY `bus-type` (1) despite being unsupported, and universally mandates `clock-lanes` which is invalid for CPHY.
- [Low] The constraint `const: 0` (and `const: 3`) on `clock-lanes` causes a type mismatch validation failure.
--

commit fe5fc3d14e64c01a85e54ac20b525d1bf21bb991
Author: Bryan O'Donoghue <bryan.odonoghue at linaro.org>
dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema

Add a base schema for the MIPI CSI2 PHYs on Qualcomm SoCs supporting C-PHY
and D-PHY modes. The commit submitted describes D-PHY modes only, leaving
C-PHY for future work.

> 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..17de0701eb103
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml

[ ... ]

> +        patternProperties:
> +          "^endpoint(@[01])?$":
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +            properties:
> +              bus-type:
> +                enum:
> +                  - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> +                  - 4 # MEDIA_BUS_TYPE_CSI2_DPHY

[Severity: Medium]
The commit message mentions that C-PHY is left to future work, but does
including MEDIA_BUS_TYPE_CSI2_CPHY here explicitly enable C-PHY
configuration?

> +
> +              clock-lanes:
> +                maxItems: 1
> +
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +                items:
> +                  minimum: 1
> +                  maximum: 4
> +                description:
> +                  Data lane positions following this endpoint's clock
> +                  lane. Positions are a per-port logical numbering, not
> +                  package pad or ball positions.
> +
> +            required:
> +              - bus-type
> +              - data-lanes
> +              - clock-lanes
> +              - remote-endpoint

[Severity: Medium]
Since C-PHY does not use a clock lane, does mandating clock-lanes in this
required block inadvertently break any potential C-PHY configurations
that might use bus-type = <1>?

> +
> +        allOf:
> +          - if:
> +              required:
> +                - endpoint at 1
> +            then:
> +              properties:
> +                endpoint at 0:
> +                  properties:
> +                    clock-lanes:
> +                      const: 0

[Severity: Low]
Since clock-lanes is parsed from the device tree as a JSON array (like [0]),
will using a scalar integer const: 0 here cause a type mismatch validation
failure during dt_binding_check?

> +                    data-lanes:
> +                      maxItems: 2
> +                endpoint at 1:
> +                  properties:
> +                    clock-lanes:
> +                      const: 3

[Severity: Low]
Does this also need to be an array constraint instead of const: 3 to pass
schema validation?

> +                    data-lanes:
> +                      items:
> +                        - const: 4
> +              required:
> +                - endpoint at 0
> +            else:
> +              patternProperties:
> +                "^endpoint(@0)?$":
> +                  properties:
> +                    clock-lanes:
> +                      const: 0

[Severity: Low]
Similarly, does this fallback path need to constrain an array rather than
using const: 0?

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