Re: [PATCH v9 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy
Date: Thu Jul 09 2026 - 03:41:27 EST
On 7/9/26 00:55, Bryan O'Donoghue wrote:
On 08/07/2026 08:44, Vladimir Zapolskiy wrote:
On 7/8/26 02:39, Bryan O'Donoghue wrote:
Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
PHY devices.
The hardware can support both CPHY, DPHY and a special split-mode DPHY.
The schema here defines two ports with three endpoints:
port@0: Sensor input.
endpoint@0: primary sensor
endpoint@1: optional second sensor, implies DPHY split-mode
port@1: Controller output.
The CSIPHY devices have their own pinouts on the SoC as well as their own
individual voltage rails.
The need to model voltage rails on a per-PHY basis leads us to define
CSIPHY devices as individual nodes.
Two nice outcomes in terms of schema and DT arise from this change.
1. The ability to define on a per-PHY basis voltage rails.
2. The ability to require those voltage.
We have had a complete bodge upstream for this where a single set of
voltage rail for all CSIPHYs has been buried inside of CAMSS.
Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in
CAMSS parlance, the CSIPHY devices should be individually modelled.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
---
.../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 202 +++++++++++++++++++++
1 file changed, 202 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..a7fbf6804cd9e
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
@@ -0,0 +1,202 @@
+# 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 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
+
+ "#phy-cells":
+ const: 1
+ description:
+ The single cell specifies the PHY operating mode.
Unfortunately my review comment given before was ignored before publishing
this version, thus I will repeat.
There is a clash between the proposed phy cells value and 'bus-type' property
of the media endpoint, the proposed value of phy cells brings no information,
and therefore the whole proposed '#phy-cells' prorperty shall be removed.
There shall be no third link introduced between CAMSS and CAMSS CSIPHYs.
As explained in the cover letter: I'm following guidance from Rob on that.
Pehaps you've misread a given review comment, because Rob's guidance is
to remove 'phy-type' property, and so far there were no arguments given
against removing unnecessary '#phy-cells' propert, which is needed to
exclude the clash on peer's side between its value and 'bus-type' value.
20260708-x1e-csi2-phy-v9-0-0210b90c04cf@xxxxxxxxxx
Old guidance granted, so let me see if I can get his attention to this
matter. Perhaps your suggestion is fine by him if so then fine, if not I
will stick to his original nudge, either way its up to him as the
original reviewer and senior schema maintainer to call this one.
Hopefully Rob will discuss my review comment, at the moment there is no
any technical arguments against it from anyone, so it's still open.
+
+ clocks:
+ maxItems: 3
+
+ clock-names:
+ items:
+ - const: core
+ - const: timer
+ - const: ahb
+
+ interrupts:
+ maxItems: 1
+
+ operating-points-v2: true
+
+ 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
None of the power domains finds its place in this device tree node, the
child device belongs to CAMSS, which already enables these power domains.
Right, I think the whole subnode debate has run its course. Its a
problem for JPEG and a bit of a fake debate, there is no real "bus" here
so what is the point of mutating DT to look like a bus ? We are forever
lecturing people about "making fake DT stuff for convenience".
Please remind me, what is the point of grouping CAMSS devices under one
device tree node? Is there an X1E CAMSS device?
Peer nodes with TITAN_TOP_GDSC will perfectly adequately describe this
hardware and the JPEG too.
It's not an argument against my proposal to get a simpler hardware
description in the form of device tree bindings. Please address
the review comments actually received.
+
+ vdda-0p8-supply:
+ description: Phandle to a 0.8V regulator supply to a PHY.
The property name shall be vdda-0p9-supply, the description shall be
changed accordingly
Hmm what ? I'll have to look back through my notes, I thought the opposite.
Let me follow up in v10 after grepping for this again.>>
+
+ 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 one to four data lanes. DPHY split mode,
+ where two independent sensors share the same PHY, is described
+ by two endpoints; endpoint@0 with exactly two-data lanes and
+ endpoint@1 with exactly one data-lane.
+ unevaluatedProperties: false
+
+ patternProperties:
+ "^endpoint(@[0-9a-f]+)?$":
This is too wide regexp mask for one or two endpoints only.
Yeah you're right, this is just fluff.
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+ remote-endpoint: true
+ required:
+ - data-lanes
+ - remote-endpoint
+
+ allOf:
+ - if:
+ required:
+ - endpoint@1
+ then:
+ properties:
+ endpoint@0:
+ properties:
+ data-lanes:
+ minItems: 2
+ maxItems: 2
+ endpoint@1:
+ properties:
+ data-lanes:
+ maxItems: 1
+ required:
+ - endpoint@0
+
+ port@1:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ description: Output to the CAMSS CSID controller.
+ unevaluatedProperties: false
+
+ patternProperties:
+ "^endpoint(@[0-9a-f]+)?$":
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ unevaluatedProperties: false
+ properties:
+ remote-endpoint: true
+ required:
+ - remote-endpoint
+
+ required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+ - clocks
+ - clock-names
+ - interrupts
+ - operating-points-v2
+ - power-domains
+ - power-domain-names
+ - vdda-0p8-supply
+ - vdda-1p2-supply
+ - ports
+
+additionalProperties: 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>
+
+ phy@ace4000 {
+ compatible = "qcom,x1e80100-csi2-phy";
+ reg = <0x0ace4000 0x2000>;
+ #phy-cells = <1>;
+
+ 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-0p8-supply = <&vreg_l2c_0p8>;
+ vdda-1p2-supply = <&vreg_l1c_1p2>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ csiphy0_in: endpoint {
+ data-lanes = <0 1 2 3>;
The previously given review comment about numeration of lanes was
not implemented.
I understand your comment but as Nihal pointed out - CAMSS has an
established pattern for this - and I think his argument is convincing -
we should stick to that pattern.
I disagree with it, since it's a new dt binding description, and the
issue is known in advance, it should fixed and not ignored.
+ 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>;
+ };
+ };
--
Best wishes,
Vladimir