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

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 21/05/2026 13:26, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The schema unconditionally requires `clock-lanes` for `port@0`, which incorrectly breaks C-PHY configurations.
> - [Low] The example device tree node is missing required `data-lanes` and `clock-lanes` properties in the `port@0` endpoint.
> --
> 
> commit bec75335fd5214a065f090cefeb28f9c895cfd3c
> Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> 
> dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
> 
> This commit adds a new base devicetree schema for Qualcomm MIPI CSI2 PHY
> devices, initially for the x1e80100. It supports CPHY, DPHY, and DPHY split
> modes across three ports, allowing per-PHY voltage rails to be individually
> defined and required.
> 
>> 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..c9116246c1e9e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> [ ... ]
>> +    properties:
>> +      port@0:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        description: Sensor input. Always present.
>> +
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +            properties:
>> +              data-lanes:
>> +                minItems: 1
>> +                maxItems: 4
>> +              clock-lanes:
>> +                maxItems: 1
>> +              remote-endpoint: true
>> +            required:
>> +              - data-lanes
>> +              - clock-lanes
> 
> [Severity: Medium]
> The commit description mentions the PHY supports both C-PHY and D-PHY modes.
> Since C-PHY embeds the clock signal within the 3-phase data lanes, there isn't
> a separate physical clock lane. Does requiring clock-lanes unconditionally
> here break configurations that need to use C-PHY mode?

Fair hadn't thought of that.

> 
>> +              - remote-endpoint
> [ ... ]
>> +examples:
>> +  - |
> [ ... ]
>> +        ports {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            port@0 {
>> +                reg = <0>;
>> +                csiphy0_in_ep: endpoint {
>> +                    remote-endpoint = <&sensor_out>;
> 
> [Severity: Low]
> The schema definition above explicitly requires data-lanes and clock-lanes to
> be present in the port@0 endpoint. Will omitting them from this example cause
> a validation failure when running dt_binding_check?

Yes this is what happens when you ask the question "should I run the 
check one more time" and say no.

>> +                };
>> +            };
>> +
>> +            port@2 {
>> +                reg = <2>;
>> +                csiphy0_out_ep: endpoint {
>> +                    remote-endpoint = <&controller_in>;
>> +                };
>> +            };
>> +        };
>> +    };
> [ ... ]
>