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

Bryan O'Donoghue bryan.odonoghue at linaro.org
Sun May 24 03:59:09 PDT 2026


On 23/05/2026 04:04, sashiko-bot at kernel.org wrote:
> 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 at 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 at ...` instead of the standard `phy at ...`.
> - [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 at 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?

This was feedback I forgot to implement yes.

>> +
>> +  vdda-1p2-supply:
>> +    description: Phandle to 1.2V regulator supply to a PHY.
>> +
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port at 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 at 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?

Not sure about the answer to that. Also feels like the tool could have 
made this suggestion at v7, definitely room for improvement.

> 
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            unevaluatedProperties: false
> [ ... ]
>> +      port at 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 at 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.

Not sure where Sashiko is getting this "typically" stuff from since 
split-mode is a qcomism.

But no in split mode port at 1 is specifically restricted this way.

Is there a way to give definitive feedback - with a meta comment perhaps 
- that stops the tool continually giving the same feedback ?

> 
>> +              clock-lanes:
>> +                maxItems: 1
> [ ... ]
>> +      port at 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 at ace4000 {
> 
> [Severity: Low]
> Does this node name violate the standard naming convention for PHY providers?

In the example does that matter ? Passes the dt checker anyway.

> The core phy-provider.yaml schema expects node names to match the generic
> pattern ^phy(@.*)?$. The current name csiphy at ace4000 will likely trigger a
> dt_binding_check warning.
> 
>> +        compatible = "qcom,x1e80100-csi2-phy";
>> +        reg = <0x0ace4000 0x2000>;
>> +        #phy-cells = <1>;
> 




More information about the linux-phy mailing list