[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