Re: [PATCH v16 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue
Date: Thu Sep 03 2026 - 08:42:37 EST
On 03/09/2026 12:53, Vladimir Zapolskiy wrote:
On 9/1/26 23:15, Bryan O'Donoghue wrote:Well, I'm not trying to describe CPHY here so I'm not claiming it is adequately described.
Add a base schema for the MIPI CSI2 PHYs on Qualcomm SoCs. This PHY
supports both DPHY and CPHY operation. A special mode of DPHY operation -
called variously split-mode or combo-mode also allows for two sensors to be
connected to one PHY.
The submitted binding here describes the DPHY modes of operation only. CPHY
is left to future work.
Like I've said in v14 review, the statement above is incorrect, and it has
to be changed, for whatever reason it hasn't happened so far.
From https://lore.kernel.org/all/1e1bc17a-4c7a-4c08-b0c8-2a2ff31fe870@xxxxxxxxxx/
I'll reword this.
C-PHY bus type is also well described by this version of the bindings.
Suggested-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
---
.../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 222 +++++++++++++++++++++
1 file changed, 222 insertions(+)
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
@@ -0,0 +1,222 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm X1E80100 SoC CSI2 PHY
+
+maintainers:
+ - Bryan O'Donoghue <bod@xxxxxxxxxx>
+
+description:
+ Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
+ to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
+ modes.
+
+properties:
+ compatible:
+ const: qcom,x1e80100-csi2-phy
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 3
+
+ clock-names:
+ items:
+ - const: core
+ - const: timer
+ - const: ahb
+
+ interrupts:
+ maxItems: 1
+
+ operating-points-v2: true
+
+ opp-table:
+ type: object
+
+ power-domains:
+ items:
+ - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
+ - description: MMCX voltage rail
+ - description: MXC or MXA voltage rail
+
+ power-domain-names:
+ items:
+ - const: top
+ - const: mmcx
+ - const: mx
+
+ vdda-0p9-supply:
+ description: Phandle to a 0.9V regulator supply to a PHY.
+
+ vdda-1p2-supply:
+ description: Phandle to 1.2V regulator supply to a PHY.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ 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 the clock lane at position 0 and one to four
+ data lanes.
+
+ DPHY split mode, where two independent sensors share the same PHY,
+ is described by two endpoints, each with its own clock lane position;
+ endpoint@0 with clock-lanes = <0> and one or two data-lanes and
+ endpoint@1 with clock-lanes = <3> and one data lane.
+
+ unevaluatedProperties: false
+
+ 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
+
+ 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
A review comment given to v14 is still applicable, 'bus-type' property
should be set as optional with D-PHY mode fallback.
I believe you said "could" be optional in v14 but, if you actually _want_ it optional, I'll update this to reflect, np.
+ - data-lanes
+ - clock-lanes
Here 'clock-lanes' is a truly optional proiperty since its value is
immutable in each endpoint, it should be removed from the list of
required properties.
Hmm.
I want to make sure clock-lanes is defined and set to the correct value.
For example clock-lanes = <2> for split mode is wrong.
Also 'clock-lanes' are not expected to be found in C-PHY bus type case.
Right so to both verify the property and not require it from CPHY I will make it required only for DPHY.
+ - remote-endpoint
+
+ allOf:
+ - if:
+ required:
+ - endpoint@1
+ then:
+ properties:
+ endpoint@0:
+ properties:
+ clock-lanes:
+ const: 0
+ data-lanes:
+ maxItems: 2
+ endpoint@1:
+ properties:
+ clock-lanes:
+ const: 3
+ data-lanes:
+ items:
+ - const: 4
+ required:
+ - endpoint@0
+ else:
+ patternProperties:
+ "^endpoint(@0)?$":
+ properties:
+ clock-lanes:
+ const: 0
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Output to the CAMSS CSID controller.
+
+ required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - operating-points-v2
+ - power-domains
+ - power-domain-names
+ - vdda-0p9-supply
+ - vdda-1p2-supply
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
+ #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/media/video-interfaces.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ phy@ace4000 {
+ compatible = "qcom,x1e80100-csi2-phy";
+ reg = <0x0ace4000 0x2000>;
+
+ clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+ <&camcc CAM_CC_CORE_AHB_CLK>;
+ clock-names = "core",
+ "timer",
+ "ahb";
+
+ interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
+
+ operating-points-v2 = <&csiphy_opp_table>;
+
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
+ <&rpmhpd RPMHPD_MMCX>,
+ <&rpmhpd RPMHPD_MX>;
+ power-domain-names = "top",
+ "mmcx",
+ "mx";
+
+ vdda-0p9-supply = <&vreg_l2c_0p9>;
+ vdda-1p2-supply = <&vreg_l1c_1p2>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ csiphy0_in: endpoint {
+ bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+ clock-lanes = <0>;
+ data-lanes = <1 2 3 4>;
+ remote-endpoint = <&sensor_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ csiphy0_out: endpoint {
+ remote-endpoint = <&csid_in>;
+ };
+ };
+ };
+
+ csiphy_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ required-opps = <&rpmhpd_opp_low_svs_d1>,
+ <&rpmhpd_opp_low_svs_d1>;
+ };
+ };
+ };
Apart of a few minor comments above, everything else looks good to me.
--
Best wishes,
Vladimir