[PATCH v5 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[PATCH v5 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Bryan O'Donoghue]

v5:
- Adds support to apply passed parameters for clock/data position/polarity - Neil
- Drops GEN1/GEN2 differentiation this can be reconstituted if GEN1 ever
  gets supported in this driver - Dmitry
- Drops camnoc_axi, cpas_ahb - Konrad
- Renames csiphy->core csiphy_timer->timer - Konrad
- Renames rail from 0p8 to 0p9 schematics say  VDD_A_CSI_n_0P9 - Konrad
- TITAN_TOP_GDSC dropped - Konrad
- Passes PHY_QCOM_CSI2_MODE_{DPHY|CPHY|SPLIT_DPHY} with the controller
  selecting the mode. Only DPHY mode is supported but the method to pass
  CPHY or split-mode DPHY configuration is there.
  Since split-mode is a Qualcomm specific mode the PHY modes are defined in
  our binding instead of adding a new type to include/linux/phy/phy.h - bod
- Depends-on: https://lore.kernel.org/r/20260325-dphy-params-extension-v1-0-c6df5599284a@linaro.org
- Link to v4: https://lore.kernel.org/r/20260315-x1e-csi2-phy-v4-0-90c09203888d@linaro.org

v4:
- MMCX, MCX and MX/MXA power-domains added - Dmitry, Vijay, Konrad
- power-domain-names added as required - bod
- opp-tables amended to capture RPMHPD deps - Dmitry, Vijay
- Switched to dev_pm_opp_set_rate, dev_pm_domain_attach_by_name etc
  dropped inherited CAMSS code - Dmitry
- Amended parameters structure to specify power-domain name list - bod
- Removed dead defines - Dmitry
- Noted in CSIPHY commit log intention to rework patterns of
  PHY lane configs into loops/defines/bit-fields later - Dmitry, bod
- Lowercase hex throughout - Dmitry
- The yaml and code in this driver doesn't care if the node is a
  sibling or a sub-node of CAMSS confirmed to work both ways - Dmitry, bod
- Link to v3: https://lore.kernel.org/r/20260226-x1e-csi2-phy-v3-0-11e608759410@linaro.org

v3:

- Resending this to make clear this submission is additive to x1e/Hamoa
  The existing bindings and code will continue to work 
  Bindings are added only, nothing is subtracted from existing ABI.
- Link to v2: https://lore.kernel.org/r/20260225-x1e-csi2-phy-v2-0-7756edb67ea9@linaro.org

v2:

In this updated version

- Added operating-point support
  The csiphy clock sets the OPP prior to setting the rate
  for csiphy and csiphy_timer - Konrad

- Combo mode
  Combo mode in CAMSS yaml has been added. Right now
  no code has been changed in the PHY driver to support it as
  I don't have hardware to test. In principle though it can
  be supported. - Vladimir

- CSIPHY init sequences
  I left these as their "magic number formats". With my diminished
  status as a non-qcom VPN person - I can no longer see what the bits
  map to. Moreover this is the situation any non-VPN community member
  will be in when submitting CSIPHY sequences derived from downstream.

  I think it is perfectly reasonable to take public CSIPHY init sequences
  as magic numbers. If someone with bit-level access wants to enumerate
  the bits that's fine but, it shouldn't gate in the interim. - Konrad/bod

- Sensor endpoints
  I've stuck to the format used by every other CSIPHY in upstream.
  Sensor endpoints hit the CAMSS/CSID endpoint not a endpoint in the PHY.
  Given the proposed changes to CAMSS though to support "combo mode" I
  think this should achieve the same outcome - multiple sensors on the one
  PHY without introducing endpoints into the PHY that no other CSIPHY in
  upstream currently has.

- Bitmask of enabled lanes
  Work needs to be done in the v4l2 layer to really support this.
  I propose making a separate series dedicated to non-linear bit
  interpretation after merging this so as to contain the scope of the
  series to something more bite (byte haha) sized. - Konrad/bod

- Link to v1: https://lore.kernel.org/r/20250710-x1e-csi2-phy-v1-0-74acbb5b162b@linaro.org

v1:
This short series adds a CSI2 MIPI PHY driver, initially supporting D-PHY
mode. The core logic and init sequences come directly from CAMSS and are
working on at least five separate x1e devices.

The rationale to instantiate CSI2 PHYs as standalone devices instead of as
sub-nodes of CAMSS is as follows.

1. Precedence
   CAMSS has a dedicated I2C bus called CCI Camera Control Interface.
   We model this controller as its own separate device in devicetree.
   This makes sense and CCI/I2C is a well defined bus type already modelled
   in Linux.

   MIPI CSI2 PHY devices similarly fit into a well defined separate
   bus/device structure.

   Contrast to another CAMSS component such as VFE, CSID or TPG these
   components only interact with other CAMSS inputs/outputs unlike CSIPHY
   which interacts with non-SoC components.

2. Hardware pinouts and rails
   The CSI2 PHY has its own data/clock lanes out from the SoC and indeed
   has its own incoming power-rails.

3. Other devicetree schemas
   There are several examples throughout the kernel of CSI PHYs modeled as
   standalone devices which one assumes follows the same reasoning as given
   above.

I've been working on this on-and-off since the end of April:
Link: https://lore.kernel.org/linux-media/c5cf0155-f839-4db9-b865-d39b56bb1e0a@linaro.org

There is another proposal to have the PHYs be subdevices of CAMSS but, I
believe we should go with a "full fat" PHY to match best practices in
drivers/phy/qualcomm/*.

Using the standard PHY API and the parameter passing that goes with it
allows us to move away from custom interfaces in CAMSS and to conform more
clearly to established PHY paradigms such as the QMP combo PHY.

Looking at existing compat strings I settled on
"qcom,x1e80100-mipi-csi2-combo-phy" deliberately omitting reference to the
fact the PHY is built on a four nano-meter process node, which seems to
match recent submissions to QMP PHY.

My first pass at this driver included support for the old two phase
devices:

Link: https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/a504c28d109296c93470340cfe7281231f573bcb#b6e59ed7db94c9da22e492bb03fcda6a4300983c

I realised that the device tree schema changes required to support a
comprehensive conversion of all CAMSS to this driver would be an
almost certainly be unacceptable ABI break or at the very least an enormous
amount of work and verification so I instead aimed to support just one new
SoC in the submission.

I've retained the callback indirections give us scope to add in another type of
future PHY including potentially adding in the 2PH later on.

This driver is tested and working on x1e/Hamoa and has been tested as not
breaking sc8280xp/Makena and sm8250/Kona.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Bryan O'Donoghue (2):
      dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
      phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

 .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 ++++++++
 MAINTAINERS                                        |  11 +
 drivers/phy/qualcomm/Kconfig                       |  13 +
 drivers/phy/qualcomm/Makefile                      |   5 +
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 361 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     | 298 +++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |  95 ++++++
 include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +
 8 files changed, 928 insertions(+)
---
base-commit: c824345288d11e269ce41b36c105715bc2286050
change-id: 20250710-x1e-csi2-phy-f6434b651d3a

Best regards,
-- 
Bryan O'Donoghue <bryan.odonoghue@linaro.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

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. We
capture those modes as:

- PHY_QCOM_CSI2_MODE_DPHY
- PHY_QCOM_CSI2_MODE_CPHY
- PHY_QCOM_CSI2_MODE_SPLIT_DPHY

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@linaro.org>
---
 .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 +++++++++++++++++++++
 include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +++
 2 files changed, 145 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..63114151104b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
@@ -0,0 +1,130 @@
+# 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 CSI2 PHY
+
+maintainers:
+  - Bryan O'Donoghue <bod@kernel.org>
+
+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.
+      See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: core
+      - const: timer
+
+  interrupts:
+    maxItems: 1
+
+  operating-points-v2:
+    maxItems: 1
+
+  power-domains:
+    items:
+      - description: MXC or MXA voltage rail
+      - description: MMCX voltage rail
+
+  power-domain-names:
+    items:
+      - const: mx
+      - const: mmcx
+
+  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.
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+  - clocks
+  - clock-names
+  - interrupts
+  - operating-points-v2
+  - power-domains
+  - power-domain-names
+  - vdda-0p9-supply
+  - vdda-1p2-supply
+
+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/phy/phy-qcom-mipi-csi2.h>
+    #include <dt-bindings/power/qcom,rpmhpd.h>
+
+    csiphy4: csiphy@ace4000 {
+        compatible = "qcom,x1e80100-csi2-phy";
+        reg = <0x0ace4000 0x2000>;
+        #phy-cells = <1>;
+
+        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
+                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
+        clock-names = "core",
+                      "timer";
+
+        operating-points-v2 = <&csiphy_opp_table>;
+
+        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
+
+        power-domains = <&rpmhpd RPMHPD_MX>,
+                        <&rpmhpd RPMHPD_MMCX>;
+        power-domain-names = "mx",
+                             "mmcx";
+
+        vdda-0p9-supply = <&vreg_l2c_0p8>;
+        vdda-1p2-supply = <&vreg_l1c_1p2>;
+    };
+
+    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>;
+        };
+
+        opp-400000000 {
+            opp-hz = /bits/ 64 <400000000>;
+            required-opps = <&rpmhpd_opp_low_svs>,
+                            <&rpmhpd_opp_low_svs>;
+        };
+
+        opp-480000000 {
+            opp-hz = /bits/ 64 <480000000>;
+            required-opps = <&rpmhpd_opp_low_svs>,
+                            <&rpmhpd_opp_low_svs>;
+        };
+    };
+
+    isp@acb7000 {
+        phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
+    };
diff --git a/include/dt-bindings/phy/phy-qcom-mipi-csi2.h b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
new file mode 100644
index 0000000000000..fa48fd75c58d8
--- /dev/null
+++ b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Qualcomm MIPI CSI2 PHY constants
+ *
+ * Copyright (C) 2026 Linaro Limited
+ */
+
+#ifndef __DT_BINDINGS_PHY_MIPI_CSI2__
+#define __DT_BINDINGS_PHY_MIPI_CSI2__
+
+#define PHY_QCOM_CSI2_MODE_DPHY		0
+#define PHY_QCOM_CSI2_MODE_CPHY		1
+#define PHY_QCOM_CSI2_MODE_SPLIT_DPHY	2
+
+#endif /* __DT_BINDINGS_PHY_MIPI_CSI2__ */

-- 
2.52.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Vladimir Zapolskiy]

On 3/26/26 03:04, 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. We
> capture those modes as:
> 
> - PHY_QCOM_CSI2_MODE_DPHY
> - PHY_QCOM_CSI2_MODE_CPHY
> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY

Distinction between PHY_QCOM_CSI2_MODE_DPHY and PHY_QCOM_CSI2_MODE_SPLIT_DPHY
is
1) insufficient in just this simplistic form, because the assignment of
particular lanes is also needed,
2) and under the assumption that the lane mapping is set somewhere else, then
there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
it's just DPHY, and the subtype is deductible from data-lanes property on
the consumer side.

So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
and PHY_TYPE_CPHY is needed here, those two are sufficient.

> 
> 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@linaro.org>
> ---
>   .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 +++++++++++++++++++++
>   include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +++
>   2 files changed, 145 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..63114151104b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> @@ -0,0 +1,130 @@
> +# 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 CSI2 PHY
> +
> +maintainers:
> +  - Bryan O'Donoghue <bod@kernel.org>
> +
> +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.
> +      See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.

include/dt-bindings/phy/phy.h should be good enough as it's stated above.

> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: core
> +      - const: timer
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  operating-points-v2:
> +    maxItems: 1
> +
> +  power-domains:
> +    items:
> +      - description: MXC or MXA voltage rail
> +      - description: MMCX voltage rail
> +
> +  power-domain-names:
> +    items:
> +      - const: mx
> +      - const: mmcx
> +
> +  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.
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - operating-points-v2
> +  - power-domains
> +  - power-domain-names
> +  - vdda-0p9-supply
> +  - vdda-1p2-supply
> +
> +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/phy/phy-qcom-mipi-csi2.h>
> +    #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> +    csiphy4: csiphy@ace4000 {
> +        compatible = "qcom,x1e80100-csi2-phy";
> +        reg = <0x0ace4000 0x2000>;
> +        #phy-cells = <1>;
> +
> +        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> +                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
> +        clock-names = "core",
> +                      "timer";
> +
> +        operating-points-v2 = <&csiphy_opp_table>;
> +
> +        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> +        power-domains = <&rpmhpd RPMHPD_MX>,
> +                        <&rpmhpd RPMHPD_MMCX>;
> +        power-domain-names = "mx",
> +                             "mmcx";
> +
> +        vdda-0p9-supply = <&vreg_l2c_0p8>;
> +        vdda-1p2-supply = <&vreg_l1c_1p2>;
> +    };
> +
> +    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>;
> +        };
> +
> +        opp-400000000 {
> +            opp-hz = /bits/ 64 <400000000>;
> +            required-opps = <&rpmhpd_opp_low_svs>,
> +                            <&rpmhpd_opp_low_svs>;
> +        };
> +
> +        opp-480000000 {
> +            opp-hz = /bits/ 64 <480000000>;
> +            required-opps = <&rpmhpd_opp_low_svs>,
> +                            <&rpmhpd_opp_low_svs>;
> +        };
> +    };
> +
> +    isp@acb7000 {
> +        phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
> +    };

This example is incomplete in sense that it does not include CAMSS
CSIPHY IP hardware configuration in whole.

> diff --git a/include/dt-bindings/phy/phy-qcom-mipi-csi2.h b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
> new file mode 100644
> index 0000000000000..fa48fd75c58d8
> --- /dev/null
> +++ b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
> +/*
> + * Qualcomm MIPI CSI2 PHY constants
> + *
> + * Copyright (C) 2026 Linaro Limited
> + */
> +
> +#ifndef __DT_BINDINGS_PHY_MIPI_CSI2__
> +#define __DT_BINDINGS_PHY_MIPI_CSI2__
> +
> +#define PHY_QCOM_CSI2_MODE_DPHY		0
> +#define PHY_QCOM_CSI2_MODE_CPHY		1
> +#define PHY_QCOM_CSI2_MODE_SPLIT_DPHY	2
> +
> +#endif /* __DT_BINDINGS_PHY_MIPI_CSI2__ */
> 

-- 
Best wishes,
Vladimir

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
> On 3/26/26 03:04, 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. We
>> capture those modes as:
>>
>> - PHY_QCOM_CSI2_MODE_DPHY
>> - PHY_QCOM_CSI2_MODE_CPHY
>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> 
> Distinction between PHY_QCOM_CSI2_MODE_DPHY and 
> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> is
> 1) insufficient in just this simplistic form, because the assignment of
> particular lanes is also needed,
> 2) and under the assumption that the lane mapping is set somewhere else, 
> then
> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
> it's just DPHY, and the subtype is deductible from data-lanes property on
> the consumer side.
> 
> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
> and PHY_TYPE_CPHY is needed here, those two are sufficient.

Because knowing the split-mode exists and that you have asked about how 
such a thing would be supported, I thought about how to represent that 
mode right from the start, even if we don't support it.

To support split phy we will need to pass the parameter.

So we define those parameters upfront.

> 
>>
>> 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@linaro.org>
>> ---
>>   .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 +++++++++++ 
>> ++++++++++
>>   include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +++
>>   2 files changed, 145 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..63114151104b4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>> @@ -0,0 +1,130 @@
>> +# 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 CSI2 PHY
>> +
>> +maintainers:
>> +  - Bryan O'Donoghue <bod@kernel.org>
>> +
>> +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.
>> +      See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
> 
> include/dt-bindings/phy/phy.h should be good enough as it's stated above.

While include/dt-bindings/phy/phy.h provides generic definitions for 
D-PHY and C-PHY, it does not contain a definition for Qualcomm's 
proprietary Split D-PHY mode. Because this hardware supports a 
vendor-specific operating mode, introducing a vendor-specific header to 
define that state is necessary.

This is exactly what we do with the QMP to support a similar use-case - 
the PHYs do vendor specific things, so we use vendor specific defines.

If we lock to phy.h CPHY/DPHY only then we exclude the possibility of 
say adding split-mode to an upstream SoC as the DT ABI will not then 
facilitate the mode.

> 
>> +
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    items:
>> +      - const: core
>> +      - const: timer
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  operating-points-v2:
>> +    maxItems: 1
>> +
>> +  power-domains:
>> +    items:
>> +      - description: MXC or MXA voltage rail
>> +      - description: MMCX voltage rail
>> +
>> +  power-domain-names:
>> +    items:
>> +      - const: mx
>> +      - const: mmcx
>> +
>> +  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.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - "#phy-cells"
>> +  - clocks
>> +  - clock-names
>> +  - interrupts
>> +  - operating-points-v2
>> +  - power-domains
>> +  - power-domain-names
>> +  - vdda-0p9-supply
>> +  - vdda-1p2-supply
>> +
>> +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/phy/phy-qcom-mipi-csi2.h>
>> +    #include <dt-bindings/power/qcom,rpmhpd.h>
>> +
>> +    csiphy4: csiphy@ace4000 {
>> +        compatible = "qcom,x1e80100-csi2-phy";
>> +        reg = <0x0ace4000 0x2000>;
>> +        #phy-cells = <1>;
>> +
>> +        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>> +                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>> +        clock-names = "core",
>> +                      "timer";
>> +
>> +        operating-points-v2 = <&csiphy_opp_table>;
>> +
>> +        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>> +
>> +        power-domains = <&rpmhpd RPMHPD_MX>,
>> +                        <&rpmhpd RPMHPD_MMCX>;
>> +        power-domain-names = "mx",
>> +                             "mmcx";
>> +
>> +        vdda-0p9-supply = <&vreg_l2c_0p8>;
>> +        vdda-1p2-supply = <&vreg_l1c_1p2>;
>> +    };
>> +
>> +    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>;
>> +        };
>> +
>> +        opp-400000000 {
>> +            opp-hz = /bits/ 64 <400000000>;
>> +            required-opps = <&rpmhpd_opp_low_svs>,
>> +                            <&rpmhpd_opp_low_svs>;
>> +        };
>> +
>> +        opp-480000000 {
>> +            opp-hz = /bits/ 64 <480000000>;
>> +            required-opps = <&rpmhpd_opp_low_svs>,
>> +                            <&rpmhpd_opp_low_svs>;
>> +        };
>> +    };
>> +
>> +    isp@acb7000 {
>> +        phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>> +    };
> 
> This example is incomplete in sense that it does not include CAMSS
> CSIPHY IP hardware configuration in whole.


No that's not the way examples work. You don't replicate entire nodes 
from other schemas you just give a terse reference.

> 
>> diff --git a/include/dt-bindings/phy/phy-qcom-mipi-csi2.h b/include/ 
>> dt-bindings/phy/phy-qcom-mipi-csi2.h
>> new file mode 100644
>> index 0000000000000..fa48fd75c58d8
>> --- /dev/null
>> +++ b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
>> +/*
>> + * Qualcomm MIPI CSI2 PHY constants
>> + *
>> + * Copyright (C) 2026 Linaro Limited
>> + */
>> +
>> +#ifndef __DT_BINDINGS_PHY_MIPI_CSI2__
>> +#define __DT_BINDINGS_PHY_MIPI_CSI2__
>> +
>> +#define PHY_QCOM_CSI2_MODE_DPHY        0
>> +#define PHY_QCOM_CSI2_MODE_CPHY        1
>> +#define PHY_QCOM_CSI2_MODE_SPLIT_DPHY    2
>> +
>> +#endif /* __DT_BINDINGS_PHY_MIPI_CSI2__ */
>>
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Vladimir Zapolskiy]

On 3/26/26 04:03, Bryan O'Donoghue wrote:
> On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
>> On 3/26/26 03:04, 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. We
>>> capture those modes as:
>>>
>>> - PHY_QCOM_CSI2_MODE_DPHY
>>> - PHY_QCOM_CSI2_MODE_CPHY
>>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>
>> Distinction between PHY_QCOM_CSI2_MODE_DPHY and
>> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>> is
>> 1) insufficient in just this simplistic form, because the assignment of
>> particular lanes is also needed,
>> 2) and under the assumption that the lane mapping is set somewhere else,
>> then
>> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
>> it's just DPHY, and the subtype is deductible from data-lanes property on
>> the consumer side.
>>
>> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
>> and PHY_TYPE_CPHY is needed here, those two are sufficient.
> 
> Because knowing the split-mode exists and that you have asked about how
> such a thing would be supported, I thought about how to represent that
> mode right from the start, even if we don't support it.

It is good to think about this hardware confguration in advance, however
the process of describing such hardware setup is incomplete.

> 
> To support split phy we will need to pass the parameter.

What you call "split phy" is a DPHY, and "split phy" can not be supported
by adding this parameter, because it does not provide information about
lanes, and after removing this information it is just DPHY.

> So we define those parameters upfront.

This new header file has to be removed, it does not bring anything valuable.

>>
>>>
>>> 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@linaro.org>
>>> ---
>>>    .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 +++++++++++
>>> ++++++++++
>>>    include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +++
>>>    2 files changed, 145 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..63114151104b4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>> @@ -0,0 +1,130 @@
>>> +# 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 CSI2 PHY
>>> +
>>> +maintainers:
>>> +  - Bryan O'Donoghue <bod@kernel.org>
>>> +
>>> +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.
>>> +      See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
>>
>> include/dt-bindings/phy/phy.h should be good enough as it's stated above.
> 
> While include/dt-bindings/phy/phy.h provides generic definitions for
> D-PHY and C-PHY, it does not contain a definition for Qualcomm's
> proprietary Split D-PHY mode. Because this hardware supports a

What Qualcomm's proprietary Split D-PHY mode is manifested by lane mapping,
there is no need to introduce another PHY mode, it is DPHY.

> vendor-specific operating mode, introducing a vendor-specific header to
> define that state is necessary.
> 
> This is exactly what we do with the QMP to support a similar use-case -
> the PHYs do vendor specific things, so we use vendor specific defines.
> 
> If we lock to phy.h CPHY/DPHY only then we exclude the possibility of
> say adding split-mode to an upstream SoC as the DT ABI will not then
> facilitate the mode.
> 
>>
>>> +
>>> +  clocks:
>>> +    maxItems: 2
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: core
>>> +      - const: timer
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  operating-points-v2:
>>> +    maxItems: 1
>>> +
>>> +  power-domains:
>>> +    items:
>>> +      - description: MXC or MXA voltage rail
>>> +      - description: MMCX voltage rail
>>> +
>>> +  power-domain-names:
>>> +    items:
>>> +      - const: mx
>>> +      - const: mmcx
>>> +
>>> +  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.
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - "#phy-cells"
>>> +  - clocks
>>> +  - clock-names
>>> +  - interrupts
>>> +  - operating-points-v2
>>> +  - power-domains
>>> +  - power-domain-names
>>> +  - vdda-0p9-supply
>>> +  - vdda-1p2-supply
>>> +
>>> +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/phy/phy-qcom-mipi-csi2.h>
>>> +    #include <dt-bindings/power/qcom,rpmhpd.h>
>>> +
>>> +    csiphy4: csiphy@ace4000 {
>>> +        compatible = "qcom,x1e80100-csi2-phy";
>>> +        reg = <0x0ace4000 0x2000>;
>>> +        #phy-cells = <1>;
>>> +
>>> +        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>> +                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>> +        clock-names = "core",
>>> +                      "timer";
>>> +
>>> +        operating-points-v2 = <&csiphy_opp_table>;
>>> +
>>> +        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>> +
>>> +        power-domains = <&rpmhpd RPMHPD_MX>,
>>> +                        <&rpmhpd RPMHPD_MMCX>;
>>> +        power-domain-names = "mx",
>>> +                             "mmcx";
>>> +
>>> +        vdda-0p9-supply = <&vreg_l2c_0p8>;
>>> +        vdda-1p2-supply = <&vreg_l1c_1p2>;
>>> +    };
>>> +
>>> +    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>;
>>> +        };
>>> +
>>> +        opp-400000000 {
>>> +            opp-hz = /bits/ 64 <400000000>;
>>> +            required-opps = <&rpmhpd_opp_low_svs>,
>>> +                            <&rpmhpd_opp_low_svs>;
>>> +        };
>>> +
>>> +        opp-480000000 {
>>> +            opp-hz = /bits/ 64 <480000000>;
>>> +            required-opps = <&rpmhpd_opp_low_svs>,
>>> +                            <&rpmhpd_opp_low_svs>;
>>> +        };
>>> +    };
>>> +
>>> +    isp@acb7000 {
>>> +        phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>>> +    };
>>
>> This example is incomplete in sense that it does not include CAMSS
>> CSIPHY IP hardware configuration in whole.
> 
> 
> No that's not the way examples work. You don't replicate entire nodes
> from other schemas you just give a terse reference.
> 

If so, then this example makes no sense and it'd be better to remove it.

-- 
Best wishes,
Vladimir

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 26/03/2026 10:28, Vladimir Zapolskiy wrote:
> On 3/26/26 04:03, Bryan O'Donoghue wrote:
>> On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
>>> On 3/26/26 03:04, 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. We
>>>> capture those modes as:
>>>>
>>>> - PHY_QCOM_CSI2_MODE_DPHY
>>>> - PHY_QCOM_CSI2_MODE_CPHY
>>>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>>
>>> Distinction between PHY_QCOM_CSI2_MODE_DPHY and
>>> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>> is
>>> 1) insufficient in just this simplistic form, because the assignment of
>>> particular lanes is also needed,
>>> 2) and under the assumption that the lane mapping is set somewhere else,
>>> then
>>> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
>>> it's just DPHY, and the subtype is deductible from data-lanes property on
>>> the consumer side.
>>>
>>> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
>>> and PHY_TYPE_CPHY is needed here, those two are sufficient.
>>
>> Because knowing the split-mode exists and that you have asked about how
>> such a thing would be supported, I thought about how to represent that
>> mode right from the start, even if we don't support it.
> 
> It is good to think about this hardware confguration in advance, however
> the process of describing such hardware setup is incomplete.
> 
>>
>> To support split phy we will need to pass the parameter.
> 
> What you call "split phy" is a DPHY, and "split phy" can not be supported
> by adding this parameter, because it does not provide information about
> lanes, and after removing this information it is just DPHY.

That's just not true. If you read the camx source code you can see 
split/combo mode 2+1 1+1 data/clock mode requires special programming of 
the PHY to support.

https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b285

There is disjunction all over this file depending on the mode.

https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b767

And besides, think about it - you need different init sequences if one 
of the lanes is clock instead of data...

If we use phy.h::PHY_TYPE_DPHY then that means to support split-mode in 
the future we need to get that mode represented in phy.h - but really 
this fixed split mode isn't a generic CSI2 PHY mode, its a Qualcommism.

Nothing wrong with that - but then the mode should reflect the fact it 
is vendor specific and we absolutely 100% have to do different things in 
the PHY driver whether we are in regular DPHY mode or in split DPHY mode.

If we use PHY_TYPE_DPHY as I did in the previous patch then we can't 
convert to a vendor type later on as its an ABI break.

So we have both a sound technical reason hardware will require it to 
differentiate between DPHY and split-mode DPHY - and we also don't want 
to be bound to phy.h and then try to upstream a new DPHY_SPLIT_MODE here 
which a reviewer might reasonably say "why is this special mode from a 
specific vendor driving new defines in a shared file"

> 
>> So we define those parameters upfront.
> 
> This new header file has to be removed, it does not bring anything valuable.
> 
>>>
>>>>
>>>> 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@linaro.org>
>>>> ---
>>>>     .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 +++++++++++
>>>> ++++++++++
>>>>     include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +++
>>>>     2 files changed, 145 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..63114151104b4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>> @@ -0,0 +1,130 @@
>>>> +# 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 CSI2 PHY
>>>> +
>>>> +maintainers:
>>>> +  - Bryan O'Donoghue <bod@kernel.org>
>>>> +
>>>> +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.
>>>> +      See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
>>>
>>> include/dt-bindings/phy/phy.h should be good enough as it's stated above.
>>
>> While include/dt-bindings/phy/phy.h provides generic definitions for
>> D-PHY and C-PHY, it does not contain a definition for Qualcomm's
>> proprietary Split D-PHY mode. Because this hardware supports a
> 
> What Qualcomm's proprietary Split D-PHY mode is manifested by lane mapping,
> there is no need to introduce another PHY mode, it is DPHY.
> 
>> vendor-specific operating mode, introducing a vendor-specific header to
>> define that state is necessary.
>>
>> This is exactly what we do with the QMP to support a similar use-case -
>> the PHYs do vendor specific things, so we use vendor specific defines.
>>
>> If we lock to phy.h CPHY/DPHY only then we exclude the possibility of
>> say adding split-mode to an upstream SoC as the DT ABI will not then
>> facilitate the mode.
>>
>>>
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 2
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: core
>>>> +      - const: timer
>>>> +
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +
>>>> +  operating-points-v2:
>>>> +    maxItems: 1
>>>> +
>>>> +  power-domains:
>>>> +    items:
>>>> +      - description: MXC or MXA voltage rail
>>>> +      - description: MMCX voltage rail
>>>> +
>>>> +  power-domain-names:
>>>> +    items:
>>>> +      - const: mx
>>>> +      - const: mmcx
>>>> +
>>>> +  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.
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +  - "#phy-cells"
>>>> +  - clocks
>>>> +  - clock-names
>>>> +  - interrupts
>>>> +  - operating-points-v2
>>>> +  - power-domains
>>>> +  - power-domain-names
>>>> +  - vdda-0p9-supply
>>>> +  - vdda-1p2-supply
>>>> +
>>>> +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/phy/phy-qcom-mipi-csi2.h>
>>>> +    #include <dt-bindings/power/qcom,rpmhpd.h>
>>>> +
>>>> +    csiphy4: csiphy@ace4000 {
>>>> +        compatible = "qcom,x1e80100-csi2-phy";
>>>> +        reg = <0x0ace4000 0x2000>;
>>>> +        #phy-cells = <1>;
>>>> +
>>>> +        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>> +                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>> +        clock-names = "core",
>>>> +                      "timer";
>>>> +
>>>> +        operating-points-v2 = <&csiphy_opp_table>;
>>>> +
>>>> +        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>> +
>>>> +        power-domains = <&rpmhpd RPMHPD_MX>,
>>>> +                        <&rpmhpd RPMHPD_MMCX>;
>>>> +        power-domain-names = "mx",
>>>> +                             "mmcx";
>>>> +
>>>> +        vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>> +        vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>> +    };
>>>> +
>>>> +    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>;
>>>> +        };
>>>> +
>>>> +        opp-400000000 {
>>>> +            opp-hz = /bits/ 64 <400000000>;
>>>> +            required-opps = <&rpmhpd_opp_low_svs>,
>>>> +                            <&rpmhpd_opp_low_svs>;
>>>> +        };
>>>> +
>>>> +        opp-480000000 {
>>>> +            opp-hz = /bits/ 64 <480000000>;
>>>> +            required-opps = <&rpmhpd_opp_low_svs>,
>>>> +                            <&rpmhpd_opp_low_svs>;
>>>> +        };
>>>> +    };
>>>> +
>>>> +    isp@acb7000 {
>>>> +        phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>>>> +    };
>>>
>>> This example is incomplete in sense that it does not include CAMSS
>>> CSIPHY IP hardware configuration in whole.
>>
>>
>> No that's not the way examples work. You don't replicate entire nodes
>> from other schemas you just give a terse reference.
>>
> 
> If so, then this example makes no sense and it'd be better to remove it.
You know you're right its not strictly necessary - its just there to be 
helpful.

"Be less helpful" is not usually review feedback I give or take but, 
I'll drop this anyway.

---
bod


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Vladimir Zapolskiy]

On 3/26/26 16:42, Bryan O'Donoghue wrote:
> On 26/03/2026 10:28, Vladimir Zapolskiy wrote:
>> On 3/26/26 04:03, Bryan O'Donoghue wrote:
>>> On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
>>>> On 3/26/26 03:04, 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. We
>>>>> capture those modes as:
>>>>>
>>>>> - PHY_QCOM_CSI2_MODE_DPHY
>>>>> - PHY_QCOM_CSI2_MODE_CPHY
>>>>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>>>
>>>> Distinction between PHY_QCOM_CSI2_MODE_DPHY and
>>>> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>>> is
>>>> 1) insufficient in just this simplistic form, because the assignment of
>>>> particular lanes is also needed,
>>>> 2) and under the assumption that the lane mapping is set somewhere else,
>>>> then
>>>> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
>>>> it's just DPHY, and the subtype is deductible from data-lanes property on
>>>> the consumer side.
>>>>
>>>> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
>>>> and PHY_TYPE_CPHY is needed here, those two are sufficient.
>>>
>>> Because knowing the split-mode exists and that you have asked about how
>>> such a thing would be supported, I thought about how to represent that
>>> mode right from the start, even if we don't support it.
>>
>> It is good to think about this hardware confguration in advance, however
>> the process of describing such hardware setup is incomplete.
>>
>>>
>>> To support split phy we will need to pass the parameter.
>>
>> What you call "split phy" is a DPHY, and "split phy" can not be supported
>> by adding this parameter, because it does not provide information about
>> lanes, and after removing this information it is just DPHY.
> 
> That's just not true. If you read the camx source code you can see
> split/combo mode 2+1 1+1 data/clock mode requires special programming of
> the PHY to support.

Please do not reduce the upraised problem of proper hardware description
to some particular realisation in camx, this is irrelevant.

Here the description of hardware is done, and my point is that the new
PHY_QCOM_CSI2_MODE_SPLIT_DPHY phy type is simply not needed, since it's
possible to give a proper description of hardware without this invention.

> https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b285
> 
> There is disjunction all over this file depending on the mode.
> 
> https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b767
> 
> And besides, think about it - you need different init sequences if one
> of the lanes is clock instead of data...
> 
> If we use phy.h::PHY_TYPE_DPHY then that means to support split-mode in
> the future we need to get that mode represented in phy.h - but really
> this fixed split mode isn't a generic CSI2 PHY mode, its a Qualcommism.
> 
> Nothing wrong with that - but then the mode should reflect the fact it
> is vendor specific and we absolutely 100% have to do different things in
> the PHY driver whether we are in regular DPHY mode or in split DPHY mode.
> 
> If we use PHY_TYPE_DPHY as I did in the previous patch then we can't
> convert to a vendor type later on as its an ABI break.
> 
> So we have both a sound technical reason hardware will require it to
> differentiate between DPHY and split-mode DPHY - and we also don't want
> to be bound to phy.h and then try to upstream a new DPHY_SPLIT_MODE here
> which a reviewer might reasonably say "why is this special mode from a
> specific vendor driving new defines in a shared file"
> 
>>
>>> So we define those parameters upfront.
>>
>> This new header file has to be removed, it does not bring anything valuable.
>>
>>>>
>>>>>
>>>>> 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@linaro.org>
>>>>> ---
>>>>>      .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 +++++++++++
>>>>> ++++++++++
>>>>>      include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +++
>>>>>      2 files changed, 145 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..63114151104b4
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>>> @@ -0,0 +1,130 @@
>>>>> +# 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 CSI2 PHY
>>>>> +
>>>>> +maintainers:
>>>>> +  - Bryan O'Donoghue <bod@kernel.org>
>>>>> +
>>>>> +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.
>>>>> +      See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
>>>>
>>>> include/dt-bindings/phy/phy.h should be good enough as it's stated above.
>>>
>>> While include/dt-bindings/phy/phy.h provides generic definitions for
>>> D-PHY and C-PHY, it does not contain a definition for Qualcomm's
>>> proprietary Split D-PHY mode. Because this hardware supports a
>>
>> What Qualcomm's proprietary Split D-PHY mode is manifested by lane mapping,
>> there is no need to introduce another PHY mode, it is DPHY.
>>
>>> vendor-specific operating mode, introducing a vendor-specific header to
>>> define that state is necessary.
>>>
>>> This is exactly what we do with the QMP to support a similar use-case -
>>> the PHYs do vendor specific things, so we use vendor specific defines.
>>>
>>> If we lock to phy.h CPHY/DPHY only then we exclude the possibility of
>>> say adding split-mode to an upstream SoC as the DT ABI will not then
>>> facilitate the mode.
>>>
>>>>
>>>>> +
>>>>> +  clocks:
>>>>> +    maxItems: 2
>>>>> +
>>>>> +  clock-names:
>>>>> +    items:
>>>>> +      - const: core
>>>>> +      - const: timer
>>>>> +
>>>>> +  interrupts:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  operating-points-v2:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  power-domains:
>>>>> +    items:
>>>>> +      - description: MXC or MXA voltage rail
>>>>> +      - description: MMCX voltage rail
>>>>> +
>>>>> +  power-domain-names:
>>>>> +    items:
>>>>> +      - const: mx
>>>>> +      - const: mmcx
>>>>> +
>>>>> +  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.
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - reg
>>>>> +  - "#phy-cells"
>>>>> +  - clocks
>>>>> +  - clock-names
>>>>> +  - interrupts
>>>>> +  - operating-points-v2
>>>>> +  - power-domains
>>>>> +  - power-domain-names
>>>>> +  - vdda-0p9-supply
>>>>> +  - vdda-1p2-supply
>>>>> +
>>>>> +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/phy/phy-qcom-mipi-csi2.h>
>>>>> +    #include <dt-bindings/power/qcom,rpmhpd.h>
>>>>> +
>>>>> +    csiphy4: csiphy@ace4000 {
>>>>> +        compatible = "qcom,x1e80100-csi2-phy";
>>>>> +        reg = <0x0ace4000 0x2000>;
>>>>> +        #phy-cells = <1>;
>>>>> +
>>>>> +        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>>> +                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>>> +        clock-names = "core",
>>>>> +                      "timer";
>>>>> +
>>>>> +        operating-points-v2 = <&csiphy_opp_table>;
>>>>> +
>>>>> +        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>>> +
>>>>> +        power-domains = <&rpmhpd RPMHPD_MX>,
>>>>> +                        <&rpmhpd RPMHPD_MMCX>;
>>>>> +        power-domain-names = "mx",
>>>>> +                             "mmcx";
>>>>> +
>>>>> +        vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>>> +        vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>>> +    };
>>>>> +
>>>>> +    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>;
>>>>> +        };
>>>>> +
>>>>> +        opp-400000000 {
>>>>> +            opp-hz = /bits/ 64 <400000000>;
>>>>> +            required-opps = <&rpmhpd_opp_low_svs>,
>>>>> +                            <&rpmhpd_opp_low_svs>;
>>>>> +        };
>>>>> +
>>>>> +        opp-480000000 {
>>>>> +            opp-hz = /bits/ 64 <480000000>;
>>>>> +            required-opps = <&rpmhpd_opp_low_svs>,
>>>>> +                            <&rpmhpd_opp_low_svs>;
>>>>> +        };
>>>>> +    };
>>>>> +
>>>>> +    isp@acb7000 {
>>>>> +        phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>>>>> +    };
>>>>
>>>> This example is incomplete in sense that it does not include CAMSS
>>>> CSIPHY IP hardware configuration in whole.
>>>
>>>
>>> No that's not the way examples work. You don't replicate entire nodes
>>> from other schemas you just give a terse reference.
>>>
>>
>> If so, then this example makes no sense and it'd be better to remove it.
> You know you're right its not strictly necessary - its just there to be
> helpful.
> 
> "Be less helpful" is not usually review feedback I give or take but,
> I'll drop this anyway.
> 

Thank you.

-- 
Best wishes,
Vladimir

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 26/03/2026 14:49, Vladimir Zapolskiy wrote:
> Here the description of hardware is done, and my point is that the new
> PHY_QCOM_CSI2_MODE_SPLIT_DPHY phy type is simply not needed, since it's
> possible to give a proper description of hardware without this invention.

Perhaps I'm not understanding you.

If we use PHY_TYPE_DPHY

include/dt-bindings/phy/phy.h:#define PHY_TYPE_DPHY		10

We _must_ then add SPLIT_MODE to phy.h if/when we implement that 
support. Which means successfully arguing the toss of weather SPLIT_MODE 
is a Qualcommism - a vendor specific mode or not.

<&phy PHY_TYPE_DPHY> committed to an upstream dts will then need to be 
supported perpetually.

So for example qrb5615 - kona/rb5 support split mode.

Pretend go with <&phy PHY_TYPE_DPHY>; and retrofit individual PHY 
support to this platform.

Grand so far.

The pretend we want to switch from one sensor to a split-mode sensor on 
the existing mezzanine.

Then we need a representation of split mode in phy.h to represent that 
in DT.

<&phy PHY_TYPE_DPHY_SPLIT_MODE>;

Except split-mode is not an appropriate mode to define in phy.h since it 
is vendor specific - even if a few vendors support it, its not a generic 
PHY mode.

Hence we would have an enormously difficult time justifying adding that 
mode to phy.h and rightly so.

>> https://review.lineageos.org/c/LineageOS/ 
>> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/ 
>> cam_csiphy/cam_csiphy_core.c#b285
>>
>> There is disjunction all over this file depending on the mode.
>>
>> https://review.lineageos.org/c/LineageOS/ 
>> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/ 
>> cam_csiphy/cam_csiphy_core.c#b767


OTOH

- SPLIT_MODE will certainly require _both_ separate init sequences
   and specific logical disjunction for additional configuration steps
   lane-assignment and masking, etc.

- That phy.h isn't the right location for SPLIT_MODE as its vendor
   specific. Just look at the modes we have for the USB PHYs
   same logic => include/dt-bindings/phy/phy-qcom-qmp.h same
   raison d'être

- And that specifying PHY_TYPE_DPHY now binds us into an ABI that we
   cannot subsequently change - it will not be possible to introduce
   include/dt-bindings/phy/phy-qcom-mipi-csi2.h later on with our mode

So therefore include/dt-bindings/phy/phy-qcom-mipi-csi2.h + PHY modes is 
the logical outcome.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Vladimir Zapolskiy]

On 3/27/26 03:03, Bryan O'Donoghue wrote:
> On 26/03/2026 14:49, Vladimir Zapolskiy wrote:
>> Here the description of hardware is done, and my point is that the new
>> PHY_QCOM_CSI2_MODE_SPLIT_DPHY phy type is simply not needed, since it's
>> possible to give a proper description of hardware without this invention.
> 
> Perhaps I'm not understanding you.

You are welcome to ask questions, it may save time.

> If we use PHY_TYPE_DPHY
> 
> include/dt-bindings/phy/phy.h:#define PHY_TYPE_DPHY		10
> 
> We _must_ then add SPLIT_MODE to phy.h if/when we implement that
> support.

I don't think it is the must.

> Which means successfully arguing the toss of weather SPLIT_MODE
> is a Qualcommism - a vendor specific mode or not.
> 
> <&phy PHY_TYPE_DPHY> committed to an upstream dts will then need to be
> supported perpetually.
> 

First of all, nobody says/defines that the phy type is mandatory to be
present in the cell at all, for instance it could be provided over bus-type
property of media endpoints, and a connected sensor unavoidably postulates
the value of this property.

> So for example qrb5615 - kona/rb5 support split mode.
> 
> Pretend go with <&phy PHY_TYPE_DPHY>; and retrofit individual PHY
> support to this platform.
> 
> Grand so far.
> 
> The pretend we want to switch from one sensor to a split-mode sensor on
> the existing mezzanine.

You may think how it should be done, it's been asked for a while to provide
a complete valid example, it may help you to get a better understanding of
the whole picture.

> 
> Then we need a representation of split mode in phy.h to represent that
> in DT.

Some kind of split mode representation should be in DT, it does not
mean that it sticks to phy.h or whatever else. Lanes (and bus-type) are
described under endpoint device tree nodes, this is totally sufficient
to separate what you call "a split mode". So, it excludes phy.h.

> 
> <&phy PHY_TYPE_DPHY_SPLIT_MODE>;
> 
> Except split-mode is not an appropriate mode to define in phy.h since it
> is vendor specific - even if a few vendors support it, its not a generic
> PHY mode.
> 
> Hence we would have an enormously difficult time justifying adding that
> mode to phy.h and rightly so.

We still discuss a hardware description, it should not be problematic to
provide descriptions of regular DPHY and what you call 'split mode' DPHY
without any new extentions of the existing dt bindings.

>>> https://review.lineageos.org/c/LineageOS/
>>> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/
>>> cam_csiphy/cam_csiphy_core.c#b285
>>>
>>> There is disjunction all over this file depending on the mode.
>>>
>>> https://review.lineageos.org/c/LineageOS/
>>> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/
>>> cam_csiphy/cam_csiphy_core.c#b767
> 
> 
> OTOH
> 
> - SPLIT_MODE will certainly require _both_ separate init sequences
>     and specific logical disjunction for additional configuration steps
>     lane-assignment and masking, etc.
> 
> - That phy.h isn't the right location for SPLIT_MODE as its vendor
>     specific. Just look at the modes we have for the USB PHYs
>     same logic => include/dt-bindings/phy/phy-qcom-qmp.h same
>     raison d'être
> 
> - And that specifying PHY_TYPE_DPHY now binds us into an ABI that we
>     cannot subsequently change - it will not be possible to introduce
>     include/dt-bindings/phy/phy-qcom-mipi-csi2.h later on with our mode
> 
> So therefore include/dt-bindings/phy/phy-qcom-mipi-csi2.h + PHY modes is
> the logical outcome.
> 

Unnecessary extention of the dt bindings ABI is not needed to complete
the task.

-- 
Best wishes,
Vladimir

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Dmitry Baryshkov]

On Thu, Mar 26, 2026 at 02:42:10PM +0000, Bryan O'Donoghue wrote:
> On 26/03/2026 10:28, Vladimir Zapolskiy wrote:
> > On 3/26/26 04:03, Bryan O'Donoghue wrote:
> > > On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
> > > > On 3/26/26 03:04, 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. We
> > > > > capture those modes as:
> > > > > 
> > > > > - PHY_QCOM_CSI2_MODE_DPHY
> > > > > - PHY_QCOM_CSI2_MODE_CPHY
> > > > > - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> > > > 
> > > > Distinction between PHY_QCOM_CSI2_MODE_DPHY and
> > > > PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> > > > is
> > > > 1) insufficient in just this simplistic form, because the assignment of
> > > > particular lanes is also needed,
> > > > 2) and under the assumption that the lane mapping is set somewhere else,
> > > > then
> > > > there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
> > > > it's just DPHY, and the subtype is deductible from data-lanes property on
> > > > the consumer side.
> > > > 
> > > > So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
> > > > and PHY_TYPE_CPHY is needed here, those two are sufficient.
> > > 
> > > Because knowing the split-mode exists and that you have asked about how
> > > such a thing would be supported, I thought about how to represent that
> > > mode right from the start, even if we don't support it.
> > 
> > It is good to think about this hardware confguration in advance, however
> > the process of describing such hardware setup is incomplete.
> > 
> > > 
> > > To support split phy we will need to pass the parameter.
> > 
> > What you call "split phy" is a DPHY, and "split phy" can not be supported
> > by adding this parameter, because it does not provide information about
> > lanes, and after removing this information it is just DPHY.
> 
> That's just not true. If you read the camx source code you can see
> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
> PHY to support.

This needs to be identified from the data-lanes / clock-lanes topology.
And once you do that, there would be (probably) no difference in the
hardware definition.


In other words, I'd also ask to drop this mode from the DT. This
infromation can and should be deduced from other, already-defined
properties.


-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 27/03/2026 20:51, Dmitry Baryshkov wrote:
>> That's just not true. If you read the camx source code you can see
>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
>> PHY to support.
> This needs to be identified from the data-lanes / clock-lanes topology.
> And once you do that, there would be (probably) no difference in the
> hardware definition.
> 
> 
> In other words, I'd also ask to drop this mode from the DT. This
> infromation can and should be deduced from other, already-defined
> properties.

It still needs to be communicated to the PHY from the controller, 
however that is not a problem I am trying to solve now.

If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.

I'll aim for DPHY only and we can come back to this topic when someone 
actually tries to enable it.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Vladimir Zapolskiy]

On 3/28/26 00:29, Bryan O'Donoghue wrote:
> On 27/03/2026 20:51, Dmitry Baryshkov wrote:
>>> That's just not true. If you read the camx source code you can see
>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
>>> PHY to support.
>> This needs to be identified from the data-lanes / clock-lanes topology.
>> And once you do that, there would be (probably) no difference in the
>> hardware definition.
>>
>>
>> In other words, I'd also ask to drop this mode from the DT. This
>> infromation can and should be deduced from other, already-defined
>> properties.
> 
> It still needs to be communicated to the PHY from the controller,
> however that is not a problem I am trying to solve now.
> 
> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.
> 
> I'll aim for DPHY only and we can come back to this topic when someone
> actually tries to enable it.
> 

DPHY may be the only supported phy type in the driver, it does not matter
at this point, however it's totally essential to cover the called by you
'split mode' right from the beginning in the renewed device tree binding
descriptions of CAMSS IPs to progress further.

-- 
Best wishes,
Vladimir

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Dmitry Baryshkov]

On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote:
> On 3/28/26 00:29, Bryan O'Donoghue wrote:
> > On 27/03/2026 20:51, Dmitry Baryshkov wrote:
> > > > That's just not true. If you read the camx source code you can see
> > > > split/combo mode 2+1 1+1 data/clock mode requires special programming of the
> > > > PHY to support.
> > > This needs to be identified from the data-lanes / clock-lanes topology.
> > > And once you do that, there would be (probably) no difference in the
> > > hardware definition.
> > > 
> > > 
> > > In other words, I'd also ask to drop this mode from the DT. This
> > > infromation can and should be deduced from other, already-defined
> > > properties.
> > 
> > It still needs to be communicated to the PHY from the controller,
> > however that is not a problem I am trying to solve now.
> > 
> > If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.
> > 
> > I'll aim for DPHY only and we can come back to this topic when someone
> > actually tries to enable it.
> > 
> 
> DPHY may be the only supported phy type in the driver, it does not matter
> at this point, however it's totally essential to cover the called by you
> 'split mode' right from the beginning in the renewed device tree binding
> descriptions of CAMSS IPs to progress further.

Okay. How would we describe that there are two sensors connected to the
single PHY anyway? How would it be described with the current bindings?

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 27/03/2026 23:23, Dmitry Baryshkov wrote:
> On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote:
>> On 3/28/26 00:29, Bryan O'Donoghue wrote:
>>> On 27/03/2026 20:51, Dmitry Baryshkov wrote:
>>>>> That's just not true. If you read the camx source code you can see
>>>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
>>>>> PHY to support.
>>>> This needs to be identified from the data-lanes / clock-lanes topology.
>>>> And once you do that, there would be (probably) no difference in the
>>>> hardware definition.
>>>>
>>>>
>>>> In other words, I'd also ask to drop this mode from the DT. This
>>>> infromation can and should be deduced from other, already-defined
>>>> properties.
>>>
>>> It still needs to be communicated to the PHY from the controller,
>>> however that is not a problem I am trying to solve now.
>>>
>>> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.
>>>
>>> I'll aim for DPHY only and we can come back to this topic when someone
>>> actually tries to enable it.
>>>
>>
>> DPHY may be the only supported phy type in the driver, it does not matter
>> at this point, however it's totally essential to cover the called by you
>> 'split mode' right from the beginning in the renewed device tree binding
>> descriptions of CAMSS IPs to progress further.
> 
> Okay. How would we describe that there are two sensors connected to the
> single PHY anyway? How would it be described with the current bindings?
> 
> --
> With best wishes
> Dmitry

Assuming you add endpoints to the PHY i.e. that is what Neil appears to 
be asking for and I personally am _fine_ with that, then it should just be

port@0
port@1

if port@1 exists, you know you are in split-phy mode.

Its actually straight forward enough, really. To be clear though I can 
write that yaml - the _most_ support I'm willing to put into the PHY 
code is to detect the port@1 and say "nope not supported yet", since 
like CPHY its not.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Dmitry Baryshkov]

On Fri, Mar 27, 2026 at 11:40:51PM +0000, Bryan O'Donoghue wrote:
> On 27/03/2026 23:23, Dmitry Baryshkov wrote:
> > On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote:
> > > On 3/28/26 00:29, Bryan O'Donoghue wrote:
> > > > On 27/03/2026 20:51, Dmitry Baryshkov wrote:
> > > > > > That's just not true. If you read the camx source code you can see
> > > > > > split/combo mode 2+1 1+1 data/clock mode requires special programming of the
> > > > > > PHY to support.
> > > > > This needs to be identified from the data-lanes / clock-lanes topology.
> > > > > And once you do that, there would be (probably) no difference in the
> > > > > hardware definition.
> > > > > 
> > > > > 
> > > > > In other words, I'd also ask to drop this mode from the DT. This
> > > > > infromation can and should be deduced from other, already-defined
> > > > > properties.
> > > > 
> > > > It still needs to be communicated to the PHY from the controller,
> > > > however that is not a problem I am trying to solve now.
> > > > 
> > > > If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.
> > > > 
> > > > I'll aim for DPHY only and we can come back to this topic when someone
> > > > actually tries to enable it.
> > > > 
> > > 
> > > DPHY may be the only supported phy type in the driver, it does not matter
> > > at this point, however it's totally essential to cover the called by you
> > > 'split mode' right from the beginning in the renewed device tree binding
> > > descriptions of CAMSS IPs to progress further.
> > 
> > Okay. How would we describe that there are two sensors connected to the
> > single PHY anyway? How would it be described with the current bindings?
> > 
> > --
> > With best wishes
> > Dmitry
> 
> Assuming you add endpoints to the PHY i.e. that is what Neil appears to be
> asking for and I personally am _fine_ with that, then it should just be
> 
> port@0
> port@1
> 
> if port@1 exists, you know you are in split-phy mode.
> 
> Its actually straight forward enough, really. To be clear though I can write
> that yaml - the _most_ support I'm willing to put into the PHY code is to
> detect the port@1 and say "nope not supported yet", since like CPHY its not.

SGTM. But let's define the schema for those usecases.

> 
> ---
> bod

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Konrad Dybcio]

On 3/29/26 12:54 PM, Dmitry Baryshkov wrote:
> On Fri, Mar 27, 2026 at 11:40:51PM +0000, Bryan O'Donoghue wrote:
>> On 27/03/2026 23:23, Dmitry Baryshkov wrote:
>>> On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote:
>>>> On 3/28/26 00:29, Bryan O'Donoghue wrote:
>>>>> On 27/03/2026 20:51, Dmitry Baryshkov wrote:
>>>>>>> That's just not true. If you read the camx source code you can see
>>>>>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
>>>>>>> PHY to support.
>>>>>> This needs to be identified from the data-lanes / clock-lanes topology.
>>>>>> And once you do that, there would be (probably) no difference in the
>>>>>> hardware definition.
>>>>>>
>>>>>>
>>>>>> In other words, I'd also ask to drop this mode from the DT. This
>>>>>> infromation can and should be deduced from other, already-defined
>>>>>> properties.
>>>>>
>>>>> It still needs to be communicated to the PHY from the controller,
>>>>> however that is not a problem I am trying to solve now.
>>>>>
>>>>> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.
>>>>>
>>>>> I'll aim for DPHY only and we can come back to this topic when someone
>>>>> actually tries to enable it.
>>>>>
>>>>
>>>> DPHY may be the only supported phy type in the driver, it does not matter
>>>> at this point, however it's totally essential to cover the called by you
>>>> 'split mode' right from the beginning in the renewed device tree binding
>>>> descriptions of CAMSS IPs to progress further.
>>>
>>> Okay. How would we describe that there are two sensors connected to the
>>> single PHY anyway? How would it be described with the current bindings?
>>>
>>> --
>>> With best wishes
>>> Dmitry
>>
>> Assuming you add endpoints to the PHY i.e. that is what Neil appears to be
>> asking for and I personally am _fine_ with that, then it should just be
>>
>> port@0
>> port@1
>>
>> if port@1 exists, you know you are in split-phy mode.
>>
>> Its actually straight forward enough, really. To be clear though I can write
>> that yaml - the _most_ support I'm willing to put into the PHY code is to
>> detect the port@1 and say "nope not supported yet", since like CPHY its not.
> 
> SGTM. But let's define the schema for those usecases.

Let's perhaps also add a short example for both a single- and dual-sensor
cases in the YAML, even if there's no plans to support the latter
configuration now

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Vladimir Zapolskiy]

On 3/28/26 01:23, Dmitry Baryshkov wrote:
> On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote:
>> On 3/28/26 00:29, Bryan O'Donoghue wrote:
>>> On 27/03/2026 20:51, Dmitry Baryshkov wrote:
>>>>> That's just not true. If you read the camx source code you can see
>>>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
>>>>> PHY to support.
>>>> This needs to be identified from the data-lanes / clock-lanes topology.
>>>> And once you do that, there would be (probably) no difference in the
>>>> hardware definition.
>>>>
>>>>
>>>> In other words, I'd also ask to drop this mode from the DT. This
>>>> infromation can and should be deduced from other, already-defined
>>>> properties.
>>>
>>> It still needs to be communicated to the PHY from the controller,
>>> however that is not a problem I am trying to solve now.
>>>
>>> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it.
>>>
>>> I'll aim for DPHY only and we can come back to this topic when someone
>>> actually tries to enable it.
>>>
>>
>> DPHY may be the only supported phy type in the driver, it does not matter
>> at this point, however it's totally essential to cover the called by you
>> 'split mode' right from the beginning in the renewed device tree binding
>> descriptions of CAMSS IPs to progress further.
> 
> Okay. How would we describe that there are two sensors connected to the
> single PHY anyway? How would it be described with the current bindings?
> 

An RFC example was sent about one year ago, it allows to specify one or
two endpoints under a single phy port:

https://lore.kernel.org/linux-arm-msm/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/

-- 
Best wishes,
Vladimir

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Rob Herring (Arm)]

On Thu, 26 Mar 2026 01:04:43 +0000, 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. We
> capture those modes as:
> 
> - PHY_QCOM_CSI2_MODE_DPHY
> - PHY_QCOM_CSI2_MODE_CPHY
> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> 
> 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@linaro.org>
> ---
>  .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 130 +++++++++++++++++++++
>  include/dt-bindings/phy/phy-qcom-mipi-csi2.h       |  15 +++
>  2 files changed, 145 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.example.dts:75.21-77.11: Warning (unit_address_vs_reg): /example-0/isp@acb7000: node has a unit name, but no reg or ranges property

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260326-x1e-csi2-phy-v5-1-0c0fc7f5c01b@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Konrad Dybcio]

On 3/26/26 2:04 AM, 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. We
> capture those modes as:
> 
> - PHY_QCOM_CSI2_MODE_DPHY
> - PHY_QCOM_CSI2_MODE_CPHY
> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY

Does the _PHY_ DT node need to be aware about this upfront?

If we have some sideband signal (e.g. the sensor driver specifically
requesting C-PHY mode), we can simply throw all this complexity into
phy_mode + phy_configure_opts, all at runtime

Further, the combo/split mode may possibly be selected through
aggregation of requests.

The question remains whether the sensor should have a direct connection to
the PHY itself (i.e. phys = <&csiphyN> or of_graph straight into the PHY)
or whether it's going to be translated by the camss node (which would be
the one holding a PHY reference) - there's probably surface for adding such
negotiation logic in both places


Note this is a question and I'm not aware of all the possible combinations

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Konrad Dybcio]

On 3/27/26 11:07 AM, Konrad Dybcio wrote:
> On 3/26/26 2:04 AM, 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. We
>> capture those modes as:
>>
>> - PHY_QCOM_CSI2_MODE_DPHY
>> - PHY_QCOM_CSI2_MODE_CPHY
>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> 
> Does the _PHY_ DT node need to be aware about this upfront?
> 
> If we have some sideband signal (e.g. the sensor driver specifically
> requesting C-PHY mode), we can simply throw all this complexity into
> phy_mode + phy_configure_opts, all at runtime

Actually perhaps just phy_configure_opts, because phy_mode could be left
as "MIPI_CSI" to reduce complexity (while still carrying info about D/C
mode in the latter)

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 27/03/2026 10:07, Konrad Dybcio wrote:
> On 3/26/26 2:04 AM, 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. We
>> capture those modes as:
>>
>> - PHY_QCOM_CSI2_MODE_DPHY
>> - PHY_QCOM_CSI2_MODE_CPHY
>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> Does the_PHY_ DT node need to be aware about this upfront?

Yeah that's a fair question.

The standard model is to pass the mode via binding right now. You 
_could_ configure it @ run time in principle.

And you could even conceive of a sensor hardware that might find value 
in that - but IMO it's a 100% hypothetical use-case - you'd basically 
need an FPGA that could output CPHY, DPHY or for some reason SPLIT_MODE 
DPHY.

But that's a pretty off the wall use-case to hypothesize. Split-mode 
OTOH is a board-level physical reality which => a DT description not a 
runtime choice.

> If we have some sideband signal (e.g. the sensor driver specifically
> requesting C-PHY mode), we can simply throw all this complexity into
> phy_mode + phy_configure_opts, all at runtime

Like I say its conceivable but IMO not a real thing and unless your 
sensor is an FPGA not possible to support in real hardware.

> Further, the combo/split mode may possibly be selected through
> aggregation of requests.
> 
> The question remains whether the sensor should have a direct connection to
> the PHY itself (i.e. phys = <&csiphyN> or of_graph straight into the PHY)
> or whether it's going to be translated by the camss node (which would be
> the one holding a PHY reference) - there's probably surface for adding such
> negotiation logic in both places

To be frankly honest you can make an argument for it either way. However 
my honestly held position is analysing other upstream implementations 
connecting to the PHY means we can't make the PHY device a drivers/phy 
device - it would have to be a V4L2 device and then for me the question 
is why is that even required ?

The model we have right now, right or wrong is sensor endpoint to 
controller. Similarly the <&phy MODE_GOES_HERE> is a pattern Rob Herring 
suggested and IMO is a consistent pattern we should aim for upstream. We 
see it in latest Rockchip, Cadence.


> Note this is a question and I'm not aware of all the possible combinations
> 
> Konrad


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Neil Armstrong]

Le 27/03/2026 à 15:38, Bryan O'Donoghue a écrit :
> On 27/03/2026 10:07, Konrad Dybcio wrote:
>> On 3/26/26 2:04 AM, 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. We
>>> capture those modes as:
>>>
>>> - PHY_QCOM_CSI2_MODE_DPHY
>>> - PHY_QCOM_CSI2_MODE_CPHY
>>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>> Does the_PHY_ DT node need to be aware about this upfront?
> 
> Yeah that's a fair question.
> 
> The standard model is to pass the mode via binding right now. You _could_ configure it @ run time in principle.
> 
> And you could even conceive of a sensor hardware that might find value in that - but IMO it's a 100% hypothetical use-case - you'd basically need an FPGA that could output CPHY, DPHY or for some reason SPLIT_MODE DPHY.
> 
> But that's a pretty off the wall use-case to hypothesize. Split-mode OTOH is a board-level physical reality which => a DT description not a runtime choice.
> 
>> If we have some sideband signal (e.g. the sensor driver specifically
>> requesting C-PHY mode), we can simply throw all this complexity into
>> phy_mode + phy_configure_opts, all at runtime
> 
> Like I say its conceivable but IMO not a real thing and unless your sensor is an FPGA not possible to support in real hardware.
> 
>> Further, the combo/split mode may possibly be selected through
>> aggregation of requests.
>>
>> The question remains whether the sensor should have a direct connection to
>> the PHY itself (i.e. phys = <&csiphyN> or of_graph straight into the PHY)
>> or whether it's going to be translated by the camss node (which would be
>> the one holding a PHY reference) - there's probably surface for adding such
>> negotiation logic in both places
> 
> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?

This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.

The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline.

> 
> The model we have right now, right or wrong is sensor endpoint to controller. Similarly the <&phy MODE_GOES_HERE> is a pattern Rob Herring suggested and IMO is a consistent pattern we should aim for upstream. We see it in latest Rockchip, Cadence.

This doesn’t properly describe the data path, we got out of this classic scheme when implementing the whole USB-C complex on qualcomm SoCs where we had to describe the data flow within all the USB/DSP/DP/PHYs elements, and we have a link between the DP controller and the Combo PHY acting as a DRM bridge.
And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here.

Looking at other vendors and sticking to this is wrong, we need to solve how to describe the Qualcomm CAMSS complex with accurate representation of the data flow and hw components interconnections. The choice of kernel API to access an HW element is irrelevant in the discussion.

Starting refactoring the DT representation of CAMSS is good, starting with PHY is weird since I don't see what it does solve we can't solve already,
but you need to take in account how the CSIPHYs are connected to other HW blocks. Simply adding a "phys =" reference from CAMSS node doesn't reflect
at all which HW element will consume the PHY, and what are the link parameters between the consumer and the PHY and between the PHY and the Sensor.
Those are the whole meaning of the port/endpoint links, where we can define multiple endpoints for a same connection to, for example, define multiple sensors on a same PHY for the split-mode.

Neil

> 
> 
>> Note this is a question and I'm not aware of all the possible combinations
>>
>> Konrad
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 27/03/2026 15:28, Neil Armstrong wrote:
>> To be frankly honest you can make an argument for it either way. 
>> However my honestly held position is analysing other upstream 
>> implementations connecting to the PHY means we can't make the PHY 
>> device a drivers/phy device - it would have to be a V4L2 device and 
>> then for me the question is why is that even required ?
> 
> This is plain wrong, DT definition is different from software 
> implementation, you can do whatever you want if you describe HW accurately.

I'm not sure what point it is you are trying to make here. Are you 
trying to say drivers/phy is OK with you but you want an endpoint ? If 
so, please just say so.

I can see an argument for that hence my response to Konrad, I just don't 
see why its a Qualcomm specific argument and of course understood stuff 
bubbles up in review, we have a public debate and come to a consensus - 
that's a good thing.

However, I'd want wider buy-in and understanding that endpoints in the 
PHYs is a more accurate description of the data-flow.

We've been applying DT bindings aplenty without that so far. So we would 
establish new CSI2 PHY bindings should represent the sensor endpoints.

Is that what you want ?

> The CSIPHYs are not tied to a single "consumer" block, they can be 
> connected to different consumers at runtime, which is not something 
> classic PHY devices are designed for. So they are de facto a media 
> element in the dynamic camera pipeline.

The existing CAMSS binding and media graph are not changed by this series.

> And actually Rob Herring asked use to define the complete data flow, it 
> was a strong requirement. I don't see why we wouldn't here.

I'm implementing feedback from Rob.

https://lore.kernel.org/linux-media/20250710230846.GA44483-robh@kernel.org/

To me, here is where we stand:

- Individual nodes - we all agree that
- As sub-nodes - I think the majority agrees this Krzsztof, Dmitry
   I'm fine with it too.
- drivers/phy - I think we are accepting this is also fine ?
- endpoints should flow into the PHY and then back to the controller

I get that argument. In fact I _like_ that argument at least I like my 
conception of that argument.

I'll stipulate to that argument meaning then that, new CSI2 PHYs shall 
include endpoints for this purpose globally.

As I've said before, there's nothing Qualcomm specific about this 
discussion, really.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Neil Armstrong]

On 3/27/26 18:42, Bryan O'Donoghue wrote:
> On 27/03/2026 15:28, Neil Armstrong wrote:
>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>
>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
> 
> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.

I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.

The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/poweroff()/exit() directly from the CSIPHY media callbacks.

> 
> I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing.
> 
> However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow.

It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it...
In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware.

> 
> We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints.

We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard.
I mean this is common sense, why would we continue to stick to the current CAMSS bindings ???

> 
> Is that what you want ?
> 
>> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline.
> 
> The existing CAMSS binding and media graph are not changed by this series.

This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately.

In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software.

Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux.

> 
>> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here.
> 
> I'm implementing feedback from Rob.
> 
> https://lore.kernel.org/linux-media/20250710230846.GA44483-robh@kernel.org/

Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ?

> 
> To me, here is where we stand:
> 
> - Individual nodes - we all agree that
> - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry
>    I'm fine with it too.
> - drivers/phy - I think we are accepting this is also fine ?

Like I said this adds a supplementary API layer for no reason and will make life harder, but I don't care personally.

> - endpoints should flow into the PHY and then back to the controller
> 
> I get that argument. In fact I _like_ that argument at least I like my conception of that argument.
> 
> I'll stipulate to that argument meaning then that, new CSI2 PHYs shall include endpoints for this purpose globally.
> 
> As I've said before, there's nothing Qualcomm specific about this discussion, really.

There is, because the current Qualcomm CAMSS bindings are insufficient and should be entirely redesigned from the ground up to properly describe the HW.

Neil

> 
> ---
> bod


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 30/03/2026 08:49, Neil Armstrong wrote:
> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>> To be frankly honest you can make an argument for it either way. 
>>>> However my honestly held position is analysing other upstream 
>>>> implementations connecting to the PHY means we can't make the PHY 
>>>> device a drivers/phy device - it would have to be a V4L2 device and 
>>>> then for me the question is why is that even required ?
>>>
>>> This is plain wrong, DT definition is different from software 
>>> implementation, you can do whatever you want if you describe HW 
>>> accurately.
>>
>> I'm not sure what point it is you are trying to make here. Are you 
>> trying to say drivers/phy is OK with you but you want an endpoint ? If 
>> so, please just say so.
> 
> I'm against using the "phys = <>" property in the CAMSS to reference the 
> PHYs, a "PHY" in the classic terminology is tied to a single consumer, 
> and if it can be shared to multiple consumer you must model a mux or 
> whatever in the middle.

The CSIPHY-to-CSID routing is runtime-configurable and is already 
managed by the media controller framework.

DT describes static hardware connections. The dynamic mux is a software 
concern, not a hardware description concern.


> The PHY API as an internal software implementation is probably fine, 
> even if it makes implementation of split mode much much harder and 
> doesn't really solve anything, you can just call init()/poweron()/ 
> poweroff()/exit() directly from the CSIPHY media callbacks.

Great.

>> I can see an argument for that hence my response to Konrad, I just 
>> don't see why its a Qualcomm specific argument and of course 
>> understood stuff bubbles up in review, we have a public debate and 
>> come to a consensus - that's a good thing.
>>
>> However, I'd want wider buy-in and understanding that endpoints in the 
>> PHYs is a more accurate description of the data-flow.
> 
> It is, and it was designed for that, and extensively used in the media 
> DT representation, so I wonder here you would not use it...
> In an ideal world, you would add nodes for each CAMSS hw elements and 
> adds port/endpoints links between all nodes to describe the data graph, 
> this would be used to construct the media controller graph, and make it 
> much easier supporting new hardware.

Yes but be pragmatic Neil. The first step in making the monolith into 
sub-nodes is the CSIPHY.

Once the CSIPHY is in, we can follow on with adding new nodes that way 
IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff 
in that new way.


> 
>>
>> We've been applying DT bindings aplenty without that so far. So we 
>> would establish new CSI2 PHY bindings should represent the sensor 
>> endpoints.
> 
> We've been using a dummy representation of CAMM in a single node with 
> only endpoints connecting to the sensors and hiding all the hardware 
> layout in code, it doesn't scale and makes supporting new HW hard.
> I mean this is common sense, why would we continue to stick to the 
> current CAMSS bindings ???

We _won't_ I just don't support a big bang integration. Progressive 
changes over a longer timeline make the transition manageable, without 
accepting endless sub-standard stuff in the meantime or holding up all 
new SoC submission unless/until.

I mean there is a CAMSS meeting which I've been running for nearly a 
year now that both you and Vlad are invited to where we have been 
discussing this for months...

Anyway one conclusion of that is we want to transition to individual 
nodes for everything.

PHY is the first step which I'm taking because its easier for me as 
CAMSS maintainer to convince the CAMSS maintainer to take the relevant 
patches.

drivers/phy notwithstanding.

>>
>> Is that what you want ?
>>
>>> The CSIPHYs are not tied to a single "consumer" block, they can be 
>>> connected to different consumers at runtime, which is not something 
>>> classic PHY devices are designed for. So they are de facto a media 
>>> element in the dynamic camera pipeline.
>>
>> The existing CAMSS binding and media graph are not changed by this 
>> series.
> 
> This is not my point, I don't care about the software implementation at 
> all, I care about accurate hardware representation. Using the "phys = 
> <>" property does not describe hardware accurately.
> 
> In other words: The CSIPHY are not connected to CAMSS. This is _not_ 
> true, tying the CSIPHYs to the CAMSS block hides the real data muxing in 
> software.
> 
> Please remind DT is used by multiple operating systems, and properly 
> describing hardware in DT will help have good software support over all 
> OSes, not just Linux.
> 
>>
>>> And actually Rob Herring asked use to define the complete data flow, 
>>> it was a strong requirement. I don't see why we wouldn't here.
>>
>> I'm implementing feedback from Rob.
>>
>> https://lore.kernel.org/linux-media/20250710230846.GA44483- 
>> robh@kernel.org/
> 
> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs 
> are muxed to multiple consumers which are burried in the CAMSS code ?

I freely admit to taking the initiative of phys = <> but Neil there is 
_no_change_ to the media graph and the "mux" is a runtime configuration 
that is one register in the CSID.

You seriously want a mux device in the kernel to model one bit in a 
register ?

No.

>>
>> To me, here is where we stand:
>>
>> - Individual nodes - we all agree that
>> - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry
>>    I'm fine with it too.
>> - drivers/phy - I think we are accepting this is also fine ?
> 
> Like I said this adds a supplementary API layer for no reason and will 
> make life harder, but I don't care personally.
> 
>> - endpoints should flow into the PHY and then back to the controller
>>
>> I get that argument. In fact I _like_ that argument at least I like my 
>> conception of that argument.
>>
>> I'll stipulate to that argument meaning then that, new CSI2 PHYs shall 
>> include endpoints for this purpose globally.
>>
>> As I've said before, there's nothing Qualcomm specific about this 
>> discussion, really.
> 
> There is, because the current Qualcomm CAMSS bindings are insufficient 
> and should be entirely redesigned from the ground up to properly 
> describe the HW.

The long term plan is to do that. Discussed extensively with the Qcom 
teams delivering CAMSS, even invited community members along.

Here are the meeting notes - its all in the public domain

https://docs.google.com/document/d/1yUwWHaKLuovKVgGTzvKm68K1zS6xBONVzjOsRjDl03U/edit?tab=t.0#heading=h.mtbm7qfohwfs

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Neil Armstrong]

On 3/30/26 11:02, Bryan O'Donoghue wrote:
> On 30/03/2026 08:49, Neil Armstrong wrote:
>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>>>
>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
>>>
>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
>>
>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
> 
> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.

This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.

> 
> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.

DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.

> 
> 
>> The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/ poweroff()/exit() directly from the CSIPHY media callbacks.
> 
> Great.
> 
>>> I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing.
>>>
>>> However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow.
>>
>> It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it...
>> In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware.
> 
> Yes but be pragmatic Neil. The first step in making the monolith into sub-nodes is the CSIPHY.

I am, and I agree it's fine to do it step by step.

> 
> Once the CSIPHY is in, we can follow on with adding new nodes that way IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff in that new way.

I agree on the approach, I never said otherwise, but you need to have the big picture in mind.

When you'll have the CSID subnodes, where will you add the phys properties ? you'll keep them in the CAMSS top node ? add all CSIPHY to all CSID nodes ? this is wrong and this needs to be fixed now.

> 
> 
>>
>>>
>>> We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints.
>>
>> We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard.
>> I mean this is common sense, why would we continue to stick to the current CAMSS bindings ???
> 
> We _won't_ I just don't support a big bang integration. Progressive changes over a longer timeline make the transition manageable, without accepting endless sub-standard stuff in the meantime or holding up all new SoC submission unless/until.
> 
> I mean there is a CAMSS meeting which I've been running for nearly a year now that both you and Vlad are invited to where we have been discussing this for months...
> 
> Anyway one conclusion of that is we want to transition to individual nodes for everything.
> 
> PHY is the first step which I'm taking because its easier for me as CAMSS maintainer to convince the CAMSS maintainer to take the relevant patches.
> 
> drivers/phy notwithstanding.

As I said I agree on the progressive approach, not the PHY DT bindins approach.

> 
>>>
>>> Is that what you want ?
>>>
>>>> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline.
>>>
>>> The existing CAMSS binding and media graph are not changed by this series.
>>
>> This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately.
>>
>> In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software.
>>
>> Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux.
>>
>>>
>>>> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here.
>>>
>>> I'm implementing feedback from Rob.
>>>
>>> https://lore.kernel.org/linux-media/20250710230846.GA44483- robh@kernel.org/
>>
>> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ?
> 
> I freely admit to taking the initiative of phys = <> but Neil there is _no_change_ to the media graph and the "mux" is a runtime configuration that is one register in the CSID.

Honestly I don't care about the userspace media graph, this is a software problem and we can totally make the transition seamless if we want.

Don't limit the DT hardware description because of a software userspace ABI breakage, this approach is not the right one.

> 
> You seriously want a mux device in the kernel to model one bit in a register ?

Why not ? We have drivers that even toggles even single bit to solve those kind of situations.

Physically they're a mux to route the CSIPHY to the consumers, it's a fact... even if it's a register or a single bit.

> 
> No.
> 
>>>
>>> To me, here is where we stand:
>>>
>>> - Individual nodes - we all agree that
>>> - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry
>>>    I'm fine with it too.
>>> - drivers/phy - I think we are accepting this is also fine ?
>>
>> Like I said this adds a supplementary API layer for no reason and will make life harder, but I don't care personally.
>>
>>> - endpoints should flow into the PHY and then back to the controller
>>>
>>> I get that argument. In fact I _like_ that argument at least I like my conception of that argument.
>>>
>>> I'll stipulate to that argument meaning then that, new CSI2 PHYs shall include endpoints for this purpose globally.
>>>
>>> As I've said before, there's nothing Qualcomm specific about this discussion, really.
>>
>> There is, because the current Qualcomm CAMSS bindings are insufficient and should be entirely redesigned from the ground up to properly describe the HW.
> 
> The long term plan is to do that. Discussed extensively with the Qcom teams delivering CAMSS, even invited community members along.
> 
> Here are the meeting notes - its all in the public domain
> 
> https://docs.google.com/document/d/1yUwWHaKLuovKVgGTzvKm68K1zS6xBONVzjOsRjDl03U/edit?tab=t.0#heading=h.mtbm7qfohwfs
> 
> ---
> bod


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 30/03/2026 10:17, Neil Armstrong wrote:
> On 3/30/26 11:02, Bryan O'Donoghue wrote:
>> On 30/03/2026 08:49, Neil Armstrong wrote:
>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>>>>
>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
>>>>
>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
>>>
>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
>>
>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
> 
> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
> 
>>
>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
> 
> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.

But right now the CAMSS block is described as a single block. There is 
no CSID device in the kernel _yet_.

When we break CSID into its own block then fine, lets have a debate 
about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> 
CSIPHY there is no DT CSID device to model this to.

> 
>>
>>
>>> The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/ poweroff()/exit() directly from the CSIPHY media callbacks.
>>
>> Great.
>>
>>>> I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing.
>>>>
>>>> However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow.
>>>
>>> It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it...
>>> In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware.
>>
>> Yes but be pragmatic Neil. The first step in making the monolith into sub-nodes is the CSIPHY.
> 
> I am, and I agree it's fine to do it step by step.

Cool.

So let's talk about muxing to CSID devices, when we have CSID devices in 
the DT.

>>
>> Once the CSIPHY is in, we can follow on with adding new nodes that way IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff in that new way.
> 
> I agree on the approach, I never said otherwise, but you need to have the big picture in mind.
> 
> When you'll have the CSID subnodes, where will you add the phys properties ? you'll keep them in the CAMSS top node ? add all CSIPHY to all CSID nodes ? this is wrong and this needs to be fixed now.
> 
>>
>>
>>>
>>>>
>>>> We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints.
>>>
>>> We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard.
>>> I mean this is common sense, why would we continue to stick to the current CAMSS bindings ???
>>
>> We _won't_ I just don't support a big bang integration. Progressive changes over a longer timeline make the transition manageable, without accepting endless sub-standard stuff in the meantime or holding up all new SoC submission unless/until.
>>
>> I mean there is a CAMSS meeting which I've been running for nearly a year now that both you and Vlad are invited to where we have been discussing this for months...
>>
>> Anyway one conclusion of that is we want to transition to individual nodes for everything.
>>
>> PHY is the first step which I'm taking because its easier for me as CAMSS maintainer to convince the CAMSS maintainer to take the relevant patches.
>>
>> drivers/phy notwithstanding.
> 
> As I said I agree on the progressive approach, not the PHY DT bindins approach.
> 
>>
>>>>
>>>> Is that what you want ?
>>>>
>>>>> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline.
>>>>
>>>> The existing CAMSS binding and media graph are not changed by this series.
>>>
>>> This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately.
>>>
>>> In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software.
>>>
>>> Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux.
>>>
>>>>
>>>>> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here.
>>>>
>>>> I'm implementing feedback from Rob.
>>>>
>>>> https://lore.kernel.org/linux-media/20250710230846.GA44483- robh@kernel.org/
>>>
>>> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ?
>>
>> I freely admit to taking the initiative of phys = <> but Neil there is _no_change_ to the media graph and the "mux" is a runtime configuration that is one register in the CSID.
> 
> Honestly I don't care about the userspace media graph, this is a software problem and we can totally make the transition seamless if we want.
> 
> Don't limit the DT hardware description because of a software userspace ABI breakage, this approach is not the right one.
> 
>>
>> You seriously want a mux device in the kernel to model one bit in a register ?
> 
> Why not ? We have drivers that even toggles even single bit to solve those kind of situations.
> 
> Physically they're a mux to route the CSIPHY to the consumers, it's a fact... even if it's a register or a single bit.

That's fine. I can understanding making that case if/when CSID becomes 
its own node but, I don't think it makes sense when connecting the PHY 
back to the monolith.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Konrad Dybcio]

On 3/30/26 11:25 AM, Bryan O'Donoghue wrote:
> On 30/03/2026 10:17, Neil Armstrong wrote:
>> On 3/30/26 11:02, Bryan O'Donoghue wrote:
>>> On 30/03/2026 08:49, Neil Armstrong wrote:
>>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>>>>>
>>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
>>>>>
>>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
>>>>
>>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
>>>
>>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
>>
>> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
>>
>>>
>>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
>>
>> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
> 
> But right now the CAMSS block is described as a single block. There is no CSID device in the kernel _yet_.
> 
> When we break CSID into its own block then fine, lets have a debate about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> CSIPHY there is no DT CSID device to model this to.

Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
the resulting nodes would either look like:

// hardcoded, m may != n
csid_n: csid@1000000 {
	phys = <&csiphy_m>;
};

or

// determined at runtime
csid_n: csid@1000000 {
	phys = <&csiphy_0>,
	       [...]
	       <&csiphy_n-1>;
};

or we could store them once, centrally, in the "CAMSS_TOP" node and
pass handles around as necessary:

// camss "catalog/manager" driver/library provides CSIDn with PHYm
camss: camss@10000000 {
	phys = <&csiphy_0>,
	       [...]
	       <&csiphy_n-1>;

	csid_n: csid@1000 {
		// no PHY references
	};
};

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 30/03/2026 12:34, Konrad Dybcio wrote:
> Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
> the resulting nodes would either look like:
> 
> // hardcoded, m may != n
> csid_n: csid@1000000 {
> 	phys = <&csiphy_m>;
> };
> 

Well that would be wrong they can connect to any CSID. We'd be churning 
the user-space ABI and imposing an artificial constraint on what the hw 
can do.

> 
> // determined at runtime
> csid_n: csid@1000000 {
> 	phys = <&csiphy_0>,
> 	       [...]
> 	       <&csiphy_n-1>;
> };

This I think works well and actually maps to what the hardware can do. 
This would be where to talk more about Neil's mux.

> 
> or we could store them once, centrally, in the "CAMSS_TOP" node and
> pass handles around as necessary:
> 
> // camss "catalog/manager" driver/library provides CSIDn with PHYm
> camss: camss@10000000 {
> 	phys = <&csiphy_0>,
> 	       [...]
> 	       <&csiphy_n-1>;
> 
> 	csid_n: csid@1000 {
> 		// no PHY references
> 	};
> };
That could work too.

Either way I think this is to be hashed out when doing CSID.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Dmitry Baryshkov]

On Mon, Mar 30, 2026 at 01:34:57PM +0200, Konrad Dybcio wrote:
> On 3/30/26 11:25 AM, Bryan O'Donoghue wrote:
> > On 30/03/2026 10:17, Neil Armstrong wrote:
> >> On 3/30/26 11:02, Bryan O'Donoghue wrote:
> >>> On 30/03/2026 08:49, Neil Armstrong wrote:
> >>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
> >>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
> >>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
> >>>>>>
> >>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
> >>>>>
> >>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
> >>>>
> >>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
> >>>
> >>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
> >>
> >> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
> >>
> >>>
> >>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
> >>
> >> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
> > 
> > But right now the CAMSS block is described as a single block. There is no CSID device in the kernel _yet_.
> > 
> > When we break CSID into its own block then fine, lets have a debate about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> CSIPHY there is no DT CSID device to model this to.
> 
> Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
> the resulting nodes would either look like:
> 
> // hardcoded, m may != n
> csid_n: csid@1000000 {
> 	phys = <&csiphy_m>;
> };
> 
> or
> 
> // determined at runtime
> csid_n: csid@1000000 {
> 	phys = <&csiphy_0>,
> 	       [...]
> 	       <&csiphy_n-1>;
> };

I think the bigger problem is:

&csid_L: {
	phys = <&csiphy_M>;
};

&csid_N: {
	phys = <&csiphy_M>;
};

aka split mode.

> 
> or we could store them once, centrally, in the "CAMSS_TOP" node and
> pass handles around as necessary:
> 
> // camss "catalog/manager" driver/library provides CSIDn with PHYm
> camss: camss@10000000 {
> 	phys = <&csiphy_0>,
> 	       [...]
> 	       <&csiphy_n-1>;
> 
> 	csid_n: csid@1000 {
> 		// no PHY references
> 	};
> };
> 
> Konrad

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Bryan O'Donoghue]

On 30/03/2026 12:49, Dmitry Baryshkov wrote:
>> // determined at runtime
>> csid_n: csid@1000000 {
>> 	phys = <&csiphy_0>,
>> 	       [...]
>> 	       <&csiphy_n-1>;
>> };
> I think the bigger problem is:
> 
> &csid_L: {
> 	phys = <&csiphy_M>;
> };
> 
> &csid_N: {
> 	phys = <&csiphy_M>;
> };
> 
> aka split mode.

Depends on how you model it.

It feels like a philosophical as opposed to an engineering debate in a way.

The CSIPHY block is defined as one thing - is split mode one PHY or two ?

You could argue it either way BUT one strong argument for it being one 
PHY is - voltage rails, input clocks etc power the block.

Sure there is an esoteric mode called split or combo mode but the 
hardware block itself the thing we usually call the PHY lives as a block 
diagram as a discreet entity.

So I think split-mode really is more like

&csid_L: {
	phys = <&csiphy_0 DPHY>, <&csiphy_1 CPHY>;
};

&csid_M: {
	phys = <&csiphy_0 DPHY>, <&csiphy_1 CPHY>;
};

Recall the debate about a mux is because CSID may connect to any CSIPHY.

csiphy_0: {

	clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
		 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
	clock-names = "core",
		      "timer";

	operating-points-v2 = <&csiphy_opp_table>;

	interrupts = <GIC_SPI 1 IRQ_TYPE_EDGE_RISING>;

	power-domains = <&rpmhpd RPMHPD_MX>,
			<&rpmhpd RPMHPD_MMCX>;
	power-domain-names = "mx",
			     "mmcx";

	vdda-0p9-supply = <&vreg_xyz_0p8>;
	vdda-1p2-supply = <&vreg_qrs_1p2>;

	ports {
		port@0{};
		port@1{};
	};
};

csiphy_N: {
	clocks = <&camcc CAM_CC_CSIPHY1_CLK>,
		 <&camcc CAM_CC_CSI1PHYTIMER_CLK>;
	clock-names = "core",
		      "timer";

	operating-points-v2 = <&csiphy_opp_table>;

	interrupts = <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>;

	power-domains = <&rpmhpd RPMHPD_MX>,
			<&rpmhpd RPMHPD_MMCX>;
	power-domain-names = "mx",
			     "mmcx";

	vdda-0p9-supply = <&vreg_abc_0p8>;
	vdda-1p2-supply = <&vreg_def_1p2>;

	ports {
		port@0{};
	};
};

IMO split mode is a special mode of that hardware block, not two 
individual PHYs.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Vladimir Zapolskiy]

On 3/30/26 12:02, Bryan O'Donoghue wrote:
> On 30/03/2026 08:49, Neil Armstrong wrote:
>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>> To be frankly honest you can make an argument for it either way.
>>>>> However my honestly held position is analysing other upstream
>>>>> implementations connecting to the PHY means we can't make the PHY
>>>>> device a drivers/phy device - it would have to be a V4L2 device and
>>>>> then for me the question is why is that even required ?
>>>>
>>>> This is plain wrong, DT definition is different from software
>>>> implementation, you can do whatever you want if you describe HW
>>>> accurately.
>>>
>>> I'm not sure what point it is you are trying to make here. Are you
>>> trying to say drivers/phy is OK with you but you want an endpoint ? If
>>> so, please just say so.
>>
>> I'm against using the "phys = <>" property in the CAMSS to reference the
>> PHYs, a "PHY" in the classic terminology is tied to a single consumer,
>> and if it can be shared to multiple consumer you must model a mux or
>> whatever in the middle.
> 
> The CSIPHY-to-CSID routing is runtime-configurable and is already
> managed by the media controller framework.
> 
> DT describes static hardware connections. The dynamic mux is a software
> concern, not a hardware description concern.
> 
> 
>> The PHY API as an internal software implementation is probably fine,
>> even if it makes implementation of split mode much much harder and
>> doesn't really solve anything, you can just call init()/poweron()/
>> poweroff()/exit() directly from the CSIPHY media callbacks.
> 
> Great.
> 
>>> I can see an argument for that hence my response to Konrad, I just
>>> don't see why its a Qualcomm specific argument and of course
>>> understood stuff bubbles up in review, we have a public debate and
>>> come to a consensus - that's a good thing.
>>>
>>> However, I'd want wider buy-in and understanding that endpoints in the
>>> PHYs is a more accurate description of the data-flow.
>>
>> It is, and it was designed for that, and extensively used in the media
>> DT representation, so I wonder here you would not use it...
>> In an ideal world, you would add nodes for each CAMSS hw elements and
>> adds port/endpoints links between all nodes to describe the data graph,
>> this would be used to construct the media controller graph, and make it
>> much easier supporting new hardware.
> 
> Yes but be pragmatic Neil. The first step in making the monolith into
> sub-nodes is the CSIPHY.
> 
> Once the CSIPHY is in, we can follow on with adding new nodes that way
> IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff
> in that new way.
> 
> 
>>
>>>
>>> We've been applying DT bindings aplenty without that so far. So we
>>> would establish new CSI2 PHY bindings should represent the sensor
>>> endpoints.
>>
>> We've been using a dummy representation of CAMM in a single node with
>> only endpoints connecting to the sensors and hiding all the hardware
>> layout in code, it doesn't scale and makes supporting new HW hard.
>> I mean this is common sense, why would we continue to stick to the
>> current CAMSS bindings ???
> 
> We _won't_ I just don't support a big bang integration. Progressive
> changes over a longer timeline make the transition manageable, without
> accepting endless sub-standard stuff in the meantime or holding up all
> new SoC submission unless/until.
> 
> I mean there is a CAMSS meeting which I've been running for nearly a
> year now that both you and Vlad are invited to where we have been
> discussing this for months...

The established process of Linux kernel development is based on email
discussions, the sent changes including CSIPHY dt bindings were plainly
ignored by the maintainer, because it's a NIH material or whatever.

There was a chance to discuss CSIPHY dt bindings changes one year ago,
now it might be not a coincidence that eventually after the series of
updates the new CSIPHY dt bindings will be very close to the once
displayed ones, it took a year, but still this is a good progress IMO.

-- 
Best wishes,
Vladimir

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v5 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Bryan O'Donoghue]

Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
existing CAMSS CSI PHY init sequences are imported in order to save time
and effort in later patches.

The following devices are supported in this drop:
"qcom,x1e80100-csi2-phy"

In-line with other PHY drivers the process node is included in the name.
Data-lane and clock lane positioning and polarity selection via newly
amended struct phy_configure_opts_mipi_dphy{} is supported.

The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
DPHY is supported.

In porting some of the logic over from camss-csiphy*.c to here its also
possible to rationalise some of the code.

In particular use of regulator_bulk and clk_bulk as well as dropping the
seemingly useless and unused interrupt handler.

The PHY sequences and a lot of the logic that goes with them are well
proven in CAMSS and mature so the main thing to watch out for here is how
to get the right sequencing of regulators, clocks and register-writes.

The register init sequence table is imported verbatim from the existing
CAMSS csiphy driver. A follow-up series will rework the table to extract
the repetitive per-lane pattern into a loop.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 MAINTAINERS                                        |  11 +
 drivers/phy/qualcomm/Kconfig                       |  13 +
 drivers/phy/qualcomm/Makefile                      |   5 +
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 361 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     | 298 +++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |  95 ++++++
 6 files changed, 783 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 62ccdc72384d4..fe19722355d94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21542,6 +21542,17 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/media/qcom,*-iris.yaml
 F:	drivers/media/platform/qcom/iris/
 
+QUALCOMM MIPI CSI2 PHY DRIVER
+M:	Bryan O'Donoghue <bod@kernel.org>
+L:	linux-phy@lists.infradead.org
+L:	linux-media@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/phy/qcom,*-csi2-phy.yaml
+F:	drivers/phy/qualcomm/phy-qcom-mipi-csi2*.c
+F:	drivers/phy/qualcomm/phy-qcom-mipi-csi2*.h
+F:	include/dt-bindings/phy/phy-qcom-mipi-csi2*
+
 QUALCOMM NAND CONTROLLER DRIVER
 M:	Manivannan Sadhasivam <mani@kernel.org>
 L:	linux-mtd@lists.infradead.org
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 60a0ead127fa9..ea33025a40fd0 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -28,6 +28,19 @@ config PHY_QCOM_EDP
 	  Enable this driver to support the Qualcomm eDP PHY found in various
 	  Qualcomm chipsets.
 
+config PHY_QCOM_MIPI_CSI2
+	tristate "Qualcomm MIPI CSI2 PHY driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on OF
+	depends on COMMON_CLK
+	select GENERIC_PHY
+	select GENERIC_PHY_MIPI_DPHY
+	help
+	  Enable this to support the MIPI CSI2 PHY driver found in various
+	  Qualcomm chipsets. This PHY is used to connect MIPI CSI2
+	  camera sensors to the CSI Decoder in the Qualcomm Camera Subsystem
+	  CAMSS.
+
 config PHY_QCOM_IPQ4019_USB
 	tristate "Qualcomm IPQ4019 USB PHY driver"
 	depends on OF && (ARCH_QCOM || COMPILE_TEST)
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index b71a6a0bed3f1..382cb594b06b6 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -6,6 +6,11 @@ obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
 obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
 obj-$(CONFIG_PHY_QCOM_M31_EUSB)		+= phy-qcom-m31-eusb2.o
+
+phy-qcom-mipi-csi2-objs			+= phy-qcom-mipi-csi2-core.o \
+					   phy-qcom-mipi-csi2-3ph-dphy.o
+obj-$(CONFIG_PHY_QCOM_MIPI_CSI2)	+= phy-qcom-mipi-csi2.o
+
 obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
 
 obj-$(CONFIG_PHY_QCOM_QMP_COMBO)	+= phy-qcom-qmp-combo.o phy-qcom-qmp-usbc.o
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
new file mode 100644
index 0000000000000..b1eb2b28b2da2
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v1.0
+ *
+ * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016-2025 Linaro Ltd.
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/time64.h>
+
+#include "phy-qcom-mipi-csi2.h"
+
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n)	((offset) + 0x4 * (n))
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET	BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE	BIT(7)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B	BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID	BIT(1)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD	BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n)	((offset) + 0xb0 + 0x4 * (n))
+
+/*
+ * 3 phase CSI has 19 common status regs with only 0-10 being used
+ * and 11-18 being reserved.
+ */
+#define CSI_COMMON_STATUS_NUM				11
+/*
+ * There are a number of common control registers
+ * The offset to clear the CSIPHY IRQ status starts @ 22
+ * So to clear CSI_COMMON_STATUS0 this is CSI_COMMON_CONTROL22, STATUS1 is
+ * CONTROL23 and so on
+ */
+#define CSI_CTRL_STATUS_INDEX				22
+
+/*
+ * There are 43 COMMON_CTRL registers with regs after # 33 being reserved
+ */
+#define CSI_CTRL_MAX					33
+
+#define CSIPHY_DEFAULT_PARAMS				0
+#define CSIPHY_SETTLE_CNT_LOWER_BYTE			2
+#define CSIPHY_SKEW_CAL					7
+
+/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
+static const struct
+mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
+	/* Power up lanes 2ph mode */
+	{.reg_addr = 0x1014, .reg_data = 0xd5, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+
+	{.reg_addr = 0x0094, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x00a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0090, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0098, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0094, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0030, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0000, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0038, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x002c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0034, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x001c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0014, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x003c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0004, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0020, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0008, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0010, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0094, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x005c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0060, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0064, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+	{.reg_addr = 0x0e94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0ea0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e28, .reg_data = 0x04, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e00, .reg_data = 0x80, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e0c, .reg_data = 0xff, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e38, .reg_data = 0x1f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0e10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+
+	{.reg_addr = 0x0494, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x04a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0490, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0498, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0494, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0430, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0400, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0438, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x042c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0434, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x041c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0414, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x043c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0404, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0420, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0408, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0410, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0494, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x045c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0460, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0464, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+	{.reg_addr = 0x0894, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x08a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0890, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0898, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0894, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0830, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0800, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0838, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x082c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0834, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x081c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0814, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x083c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0804, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0820, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0808, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0810, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0894, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x085c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0860, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0864, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+	{.reg_addr = 0x0c94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0ca0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c00, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c38, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0c10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c94, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0c5c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0c60, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+};
+
+static inline const struct mipi_csi2phy_device_regs *
+csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
+{
+	return &csi2phy->soc_cfg->reg_info;
+}
+
+static void phy_qcom_mipi_csi2_hw_version_read(struct mipi_csi2phy_device *csi2phy)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+	u32 tmp;
+
+	writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 12));
+	csi2phy->hw_version = tmp;
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 13));
+	csi2phy->hw_version |= (tmp << 8) & 0xFF00;
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 14));
+	csi2phy->hw_version |= (tmp << 16) & 0xFF0000;
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 15));
+	csi2phy->hw_version |= (tmp << 24) & 0xFF000000;
+
+	dev_dbg_once(csi2phy->dev, "CSIPHY 3PH HW Version = 0x%08x\n", csi2phy->hw_version);
+}
+
+/*
+ * phy_qcom_mipi_csi2_reset - Perform software reset on CSIPHY module
+ * @phy_qcom_mipi_csi2: CSIPHY device
+ */
+static void phy_qcom_mipi_csi2_reset(struct mipi_csi2phy_device *csi2phy)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+
+	writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET,
+	       csi2phy->base + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+	usleep_range(5000, 8000);
+	writel(0x0, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+}
+
+/*
+ * phy_qcom_mipi_csi2_settle_cnt_calc - Calculate settle count value
+ *
+ * Helper function to calculate settle count value. This is
+ * based on the CSI2 T_hs_settle parameter which in turn
+ * is calculated based on the CSI2 transmitter link frequency.
+ *
+ * Return settle count value or 0 if the CSI2 link frequency
+ * is not available
+ */
+static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
+{
+	u32 t_hs_prepare_max_ps;
+	u32 timer_period_ps;
+	u32 t_hs_settle_ps;
+	u8 settle_cnt;
+	u32 ui_ps;
+
+	if (link_freq <= 0)
+		return 0;
+
+	ui_ps = div_u64(PSEC_PER_SEC, link_freq);
+	ui_ps /= 2;
+	t_hs_prepare_max_ps = 85000 + 6 * ui_ps;
+	t_hs_settle_ps = t_hs_prepare_max_ps;
+
+	timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
+	settle_cnt = t_hs_settle_ps / timer_period_ps - 6;
+
+	return settle_cnt;
+}
+
+static void
+phy_qcom_mipi_csi2_gen2_config_lanes(struct mipi_csi2phy_device *csi2phy,
+				     u8 settle_cnt)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+	const struct mipi_csi2phy_lane_regs *r = regs->init_seq;
+	int i, array_size = regs->lane_array_size;
+	u32 val;
+
+	for (i = 0; i < array_size; i++, r++) {
+		switch (r->param_type) {
+		case CSIPHY_SETTLE_CNT_LOWER_BYTE:
+			val = settle_cnt & 0xff;
+			break;
+		case CSIPHY_SKEW_CAL:
+			/* TODO: support application of skew from dt flag */
+			continue;
+		default:
+			val = r->reg_data;
+			break;
+		}
+		writel(val, csi2phy->base + r->reg_addr);
+		if (r->delay_us)
+			udelay(r->delay_us);
+	}
+}
+
+static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
+					   struct mipi_csi2phy_stream_cfg *cfg)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+	struct mipi_csi2phy_lanes_cfg *lane_cfg = &cfg->lane_cfg;
+	u8 settle_cnt;
+	u8 val;
+	int i;
+
+	settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
+
+	val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+	for (i = 0; i < cfg->num_data_lanes; i++)
+		val |= BIT(lane_cfg->data[i].pos * 2);
+
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
+
+	val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+
+	val = 0x02;
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 7));
+
+	val = 0x00;
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+
+	phy_qcom_mipi_csi2_gen2_config_lanes(csi2phy, settle_cnt);
+
+	/* IRQ_MASK registers - disable all interrupts */
+	for (i = CSI_COMMON_STATUS_NUM; i < CSI_CTRL_STATUS_INDEX; i++) {
+		writel(0, csi2phy->base +
+		       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, i));
+	}
+
+	return 0;
+}
+
+static void
+phy_qcom_mipi_csi2_lanes_disable(struct mipi_csi2phy_device *csi2phy,
+				 struct mipi_csi2phy_stream_cfg *cfg)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+
+	writel(0, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
+
+	writel(0, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+}
+
+static const struct mipi_csi2phy_hw_ops phy_qcom_mipi_csi2_ops_3ph_1_0 = {
+	.hw_version_read = phy_qcom_mipi_csi2_hw_version_read,
+	.reset = phy_qcom_mipi_csi2_reset,
+	.lanes_enable = phy_qcom_mipi_csi2_lanes_enable,
+	.lanes_disable = phy_qcom_mipi_csi2_lanes_disable,
+};
+
+static const char * const x1e_clks[] = {
+	"core",
+	"timer"
+};
+
+static const char * const x1e_supplies[] = {
+	"vdda-0p9",
+	"vdda-1p2"
+};
+
+static const char * const x1e_genpd_names[] = {
+	"mx",
+	"mmcx",
+};
+
+const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
+	.ops = &phy_qcom_mipi_csi2_ops_3ph_1_0,
+	.reg_info = {
+		.init_seq = lane_regs_x1e80100,
+		.lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
+		.common_regs_offset = 0x1000,
+	},
+	.supply_names = (const char **)x1e_supplies,
+	.num_supplies = ARRAY_SIZE(x1e_supplies),
+	.clk_names = (const char **)x1e_clks,
+	.num_clk = ARRAY_SIZE(x1e_clks),
+	.opp_clk = x1e_clks[0],
+	.timer_clk = x1e_clks[1],
+	.genpd_names = (const char **)x1e_genpd_names,
+	.num_genpd_names = ARRAY_SIZE(x1e_genpd_names),
+};
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
new file mode 100644
index 0000000000000..47acf0d586a15
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Linaro Ltd.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm_opp.h>
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/phy/phy-qcom-mipi-csi2.h>
+
+#include "phy-qcom-mipi-csi2.h"
+
+static int
+phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
+				   s64 link_freq)
+{
+	struct device *dev = csi2phy->dev;
+	unsigned long opp_rate = link_freq / 4;
+	struct dev_pm_opp *opp;
+	long timer_rate;
+	int ret;
+
+	opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
+	if (IS_ERR(opp)) {
+		dev_err(csi2phy->dev, "Couldn't find ceiling for %lld Hz\n",
+			link_freq);
+		return PTR_ERR(opp);
+	}
+
+	for (int i = 0; i < csi2phy->num_pds; i++) {
+		unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
+
+		ret = dev_pm_genpd_set_performance_state(csi2phy->pds[i], perf);
+		if (ret) {
+			dev_err(csi2phy->dev, "Couldn't set perf state %u\n",
+				perf);
+			dev_pm_opp_put(opp);
+			return ret;
+		}
+	}
+	dev_pm_opp_put(opp);
+
+	ret = dev_pm_opp_set_rate(dev, opp_rate);
+	if (ret) {
+		dev_err(csi2phy->dev, "dev_pm_opp_set_rate() fail\n");
+		return ret;
+	}
+
+	timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
+	if (timer_rate < 0)
+		return timer_rate;
+
+	ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
+	if (ret)
+		return ret;
+
+	csi2phy->timer_clk_rate = timer_rate;
+
+	return 0;
+}
+
+static int phy_qcom_mipi_csi2_configure(struct phy *phy,
+					union phy_configure_opts *opts)
+{
+	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+	struct phy_configure_opts_mipi_dphy *dphy_cfg = &opts->mipi_dphy;
+	struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
+	int ret;
+	int i;
+
+	ret = phy_mipi_dphy_config_validate(dphy_cfg);
+	if (ret)
+		return ret;
+
+	if (dphy_cfg->lanes < 1 || dphy_cfg->lanes > CSI2_MAX_DATA_LANES)
+		return -EINVAL;
+
+	stream_cfg->link_freq = dphy_cfg->hs_clk_rate;
+	stream_cfg->num_data_lanes = dphy_cfg->lanes;
+
+	for (i = 0; i < stream_cfg->num_data_lanes; i++) {
+		stream_cfg->lane_cfg.data[i].pol = dphy_cfg->lane_polarities[i];
+		stream_cfg->lane_cfg.data[i].pos = dphy_cfg->lane_positions[i];
+	}
+
+	stream_cfg->lane_cfg.clk.pol = dphy_cfg->clock_lane_polarity;
+	stream_cfg->lane_cfg.clk.pos = dphy_cfg->clock_lane_position;
+
+	return 0;
+}
+
+static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
+{
+	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+	const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
+	struct device *dev = &phy->dev;
+	int ret;
+
+	ret = regulator_bulk_enable(csi2phy->soc_cfg->num_supplies,
+				    csi2phy->supplies);
+	if (ret)
+		return ret;
+
+	ret = phy_qcom_mipi_csi2_set_clock_rates(csi2phy, csi2phy->stream_cfg.link_freq);
+	if (ret)
+		goto poweroff_phy;
+
+	ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
+				      csi2phy->clks);
+	if (ret) {
+		dev_err(dev, "failed to enable clocks, %d\n", ret);
+		goto poweroff_phy;
+	}
+
+	ops->reset(csi2phy);
+
+	ops->hw_version_read(csi2phy);
+
+	return ops->lanes_enable(csi2phy, &csi2phy->stream_cfg);
+
+poweroff_phy:
+	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
+			       csi2phy->supplies);
+
+	return ret;
+}
+
+static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
+{
+	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+	int i;
+
+	for (i = 0; i < csi2phy->num_pds; i++)
+		dev_pm_genpd_set_performance_state(csi2phy->pds[i], 0);
+
+	clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
+				   csi2phy->clks);
+	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
+			       csi2phy->supplies);
+
+	return 0;
+}
+
+static const struct phy_ops phy_qcom_mipi_csi2_ops = {
+	.configure	= phy_qcom_mipi_csi2_configure,
+	.power_on	= phy_qcom_mipi_csi2_power_on,
+	.power_off	= phy_qcom_mipi_csi2_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *qcom_csi2_phy_xlate(struct device *dev,
+				       const struct of_phandle_args *args)
+{
+	struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
+
+	if (args->args[0] != PHY_QCOM_CSI2_MODE_DPHY) {
+		dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+
+	csi2phy->phy_mode = args->args[0];
+
+	return csi2phy->phy;
+}
+
+static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
+{
+	unsigned int i, num_clk, num_supplies, num_pds;
+	struct mipi_csi2phy_device *csi2phy;
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct phy *generic_phy;
+	int ret;
+
+	csi2phy = devm_kzalloc(dev, sizeof(*csi2phy), GFP_KERNEL);
+	if (!csi2phy)
+		return -ENOMEM;
+
+	csi2phy->dev = dev;
+	dev_set_drvdata(dev, csi2phy);
+
+	csi2phy->soc_cfg = device_get_match_data(&pdev->dev);
+
+	if (!csi2phy->soc_cfg)
+		return -EINVAL;
+
+	num_clk = csi2phy->soc_cfg->num_clk;
+	csi2phy->clks = devm_kzalloc(dev, sizeof(*csi2phy->clks) * num_clk, GFP_KERNEL);
+	if (!csi2phy->clks)
+		return -ENOMEM;
+
+	num_pds = csi2phy->soc_cfg->num_genpd_names;
+	if (!num_pds)
+		return -EINVAL;
+
+	csi2phy->pds = devm_kzalloc(dev, sizeof(*csi2phy->pds) * num_pds, GFP_KERNEL);
+	if (!csi2phy->pds)
+		return -ENOMEM;
+
+	for (i = 0; i < num_pds; i++) {
+		csi2phy->pds[i] = dev_pm_domain_attach_by_name(dev,
+							       csi2phy->soc_cfg->genpd_names[i]);
+		if (IS_ERR(csi2phy->pds[i])) {
+			return dev_err_probe(dev, PTR_ERR(csi2phy->pds[i]),
+					     "Failed to attach %s\n",
+					     csi2phy->soc_cfg->genpd_names[i]);
+		}
+	}
+	csi2phy->num_pds = num_pds;
+
+	for (i = 0; i < num_clk; i++)
+		csi2phy->clks[i].id = csi2phy->soc_cfg->clk_names[i];
+
+	ret = devm_clk_bulk_get(dev, num_clk, csi2phy->clks);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get clocks\n");
+
+	csi2phy->timer_clk = devm_clk_get(dev, csi2phy->soc_cfg->timer_clk);
+	if (IS_ERR(csi2phy->timer_clk)) {
+		return dev_err_probe(dev, PTR_ERR(csi2phy->timer_clk),
+				     "Failed to get timer clock\n");
+	}
+
+	ret = devm_pm_opp_set_clkname(dev, csi2phy->soc_cfg->opp_clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set opp clkname\n");
+
+	ret = devm_pm_opp_of_add_table(dev);
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
+
+	num_supplies = csi2phy->soc_cfg->num_supplies;
+	csi2phy->supplies = devm_kzalloc(dev, sizeof(*csi2phy->supplies) * num_supplies,
+					 GFP_KERNEL);
+	if (!csi2phy->supplies)
+		return -ENOMEM;
+
+	for (i = 0; i < num_supplies; i++)
+		csi2phy->supplies[i].supply = csi2phy->soc_cfg->supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, num_supplies, csi2phy->supplies);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get regulator supplies\n");
+
+	csi2phy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(csi2phy->base))
+		return PTR_ERR(csi2phy->base);
+
+	generic_phy = devm_phy_create(dev, NULL, &phy_qcom_mipi_csi2_ops);
+	if (IS_ERR(generic_phy)) {
+		ret = PTR_ERR(generic_phy);
+		return dev_err_probe(dev, ret, "failed to create phy\n");
+	}
+	csi2phy->phy = generic_phy;
+
+	phy_set_drvdata(generic_phy, csi2phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, qcom_csi2_phy_xlate);
+	if (!IS_ERR(phy_provider))
+		dev_dbg(dev, "Registered MIPI CSI2 PHY device\n");
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id phy_qcom_mipi_csi2_of_match_table[] = {
+	{ .compatible	= "qcom,x1e80100-csi2-phy", .data = &mipi_csi2_dphy_4nm_x1e },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, phy_qcom_mipi_csi2_of_match_table);
+
+static struct platform_driver phy_qcom_mipi_csi2_driver = {
+	.probe		= phy_qcom_mipi_csi2_probe,
+	.driver = {
+		.name	= "qcom-mipi-csi2-phy",
+		.of_match_table = phy_qcom_mipi_csi2_of_match_table,
+	},
+};
+
+module_platform_driver(phy_qcom_mipi_csi2_driver);
+
+MODULE_DESCRIPTION("Qualcomm MIPI CSI2 PHY driver");
+MODULE_AUTHOR("Bryan O'Donoghue <bryan.odonoghue@linaro.org>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
new file mode 100644
index 0000000000000..27607dea412f1
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *
+ * Qualcomm MIPI CSI2 CPHY/DPHY driver
+ *
+ * Copyright (C) 2025 Linaro Ltd.
+ */
+#ifndef __PHY_QCOM_MIPI_CSI2_H__
+#define __PHY_QCOM_MIPI_CSI2_H__
+
+#include <linux/phy/phy.h>
+
+#define CSI2_MAX_DATA_LANES 4
+
+struct mipi_csi2phy_lane {
+	u8 pos;
+	u8 pol;
+};
+
+struct mipi_csi2phy_lanes_cfg {
+	struct mipi_csi2phy_lane data[CSI2_MAX_DATA_LANES];
+	struct mipi_csi2phy_lane clk;
+};
+
+struct mipi_csi2phy_stream_cfg {
+	s64 link_freq;
+	u8 num_data_lanes;
+	struct mipi_csi2phy_lanes_cfg lane_cfg;
+};
+
+struct mipi_csi2phy_device;
+
+struct mipi_csi2phy_hw_ops {
+	void (*hw_version_read)(struct mipi_csi2phy_device *csi2phy_dev);
+	void (*reset)(struct mipi_csi2phy_device *csi2phy_dev);
+	int (*lanes_enable)(struct mipi_csi2phy_device *csi2phy_dev,
+			    struct mipi_csi2phy_stream_cfg *cfg);
+	void (*lanes_disable)(struct mipi_csi2phy_device *csi2phy_dev,
+			      struct mipi_csi2phy_stream_cfg *cfg);
+};
+
+struct mipi_csi2phy_lane_regs {
+	const s32 reg_addr;
+	const s32 reg_data;
+	const u32 delay_us;
+	const u32 param_type;
+};
+
+struct mipi_csi2phy_device_regs {
+	const struct mipi_csi2phy_lane_regs *init_seq;
+	const int lane_array_size;
+	const u32 common_regs_offset;
+};
+
+struct mipi_csi2phy_soc_cfg {
+	const struct mipi_csi2phy_hw_ops *ops;
+	const struct mipi_csi2phy_device_regs reg_info;
+
+	const char ** const supply_names;
+	const unsigned int num_supplies;
+
+	const char ** const clk_names;
+	const unsigned int num_clk;
+
+	const char * const opp_clk;
+	const char * const timer_clk;
+
+	const char ** const genpd_names;
+	const unsigned int num_genpd_names;
+};
+
+struct mipi_csi2phy_device {
+	struct device *dev;
+	u8 phy_mode;
+
+	struct phy *phy;
+	void __iomem *base;
+
+	struct clk_bulk_data *clks;
+	struct clk *timer_clk;
+	u32 timer_clk_rate;
+
+	struct regulator_bulk_data *supplies;
+	struct device **pds;
+	unsigned int num_pds;
+
+	const struct mipi_csi2phy_soc_cfg *soc_cfg;
+	struct mipi_csi2phy_stream_cfg stream_cfg;
+
+	u32 hw_version;
+};
+
+extern const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e;
+
+#endif /* __PHY_QCOM_MIPI_CSI2_H__ */

-- 
2.52.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Hangxiang Ma]

On 3/26/2026 9:04 AM, Bryan O'Donoghue wrote:
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/time64.h>
> +
> +#include "phy-qcom-mipi-csi2.h"
> +
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n)	((offset) + 0x4 * (n))
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET	BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE	BIT(7)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B	BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID	BIT(1)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD	BIT(0)
> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n)	((offset) + 0xb0 + 0x4 * (n))
>
Hi Bryan, one minor observation on the following macro:

	CSIPHY_3PH_CMN_CSI_COMMON_STATUSn

The 0xb0 offset implicitly assumes a fixed distance between the
common_ctrl and common_status register blocks. This holds for the PHYs
covered by this series, but on some other platforms (e.g. Kaanapali,
Pakala) the offset differs.

That said, I think keeping this fixed value is reasonable for the scope
of the current PHY series, and it does help keep the macro set simple.
It might just be worth documenting this assumption (e.g. via a comment
or in the commit message).

Alternatively, if future PHY variants need to support different layouts,
this could be made more extensible by moving the status base offset into
the per-PHY data (similar to other register layout parameters). But I
don’t think that needs to block the current series.

Related patch before:
<https://lore.kernel.org/all/20260112-camss-extended-csiphy-macro-v2-1-ee7342f2aaf5@oss.qualcomm.com/>

Best Regards,
Hangxiang

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Konrad Dybcio]

On 3/27/26 3:23 AM, Hangxiang Ma wrote:
> On 3/26/2026 9:04 AM, Bryan O'Donoghue wrote:
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/time64.h>
>> +
>> +#include "phy-qcom-mipi-csi2.h"
>> +
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n)    ((offset) + 0x4 * (n))
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET    BIT(0)
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE    BIT(7)
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B    BIT(0)
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID    BIT(1)
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD    BIT(0)
>> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n)    ((offset) + 0xb0 + 0x4 * (n))
>>
> Hi Bryan, one minor observation on the following macro:
> 
>     CSIPHY_3PH_CMN_CSI_COMMON_STATUSn
> 
> The 0xb0 offset implicitly assumes a fixed distance between the
> common_ctrl and common_status register blocks. This holds for the PHYs
> covered by this series, but on some other platforms (e.g. Kaanapali,
> Pakala) the offset differs.
> 
> That said, I think keeping this fixed value is reasonable for the scope
> of the current PHY series, and it does help keep the macro set simple.
> It might just be worth documenting this assumption (e.g. via a comment
> or in the commit message).
> 
> Alternatively, if future PHY variants need to support different layouts,
> this could be made more extensible by moving the status base offset into
> the per-PHY data (similar to other register layout parameters). But I
> don’t think that needs to block the current series.

If the register contents are generally similar but the bit positions
and/or reg offsets differ, regmap_fields may be useful

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Dmitry Baryshkov]

On Fri, Mar 27, 2026 at 11:07:53AM +0100, Konrad Dybcio wrote:
> On 3/27/26 3:23 AM, Hangxiang Ma wrote:
> > On 3/26/2026 9:04 AM, Bryan O'Donoghue wrote:
> >> +#include <linux/delay.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/io.h>
> >> +#include <linux/time64.h>
> >> +
> >> +#include "phy-qcom-mipi-csi2.h"
> >> +
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n)    ((offset) + 0x4 * (n))
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET    BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE    BIT(7)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B    BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID    BIT(1)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD    BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n)    ((offset) + 0xb0 + 0x4 * (n))
> >>
> > Hi Bryan, one minor observation on the following macro:
> > 
> >     CSIPHY_3PH_CMN_CSI_COMMON_STATUSn
> > 
> > The 0xb0 offset implicitly assumes a fixed distance between the
> > common_ctrl and common_status register blocks. This holds for the PHYs
> > covered by this series, but on some other platforms (e.g. Kaanapali,
> > Pakala) the offset differs.
> > 
> > That said, I think keeping this fixed value is reasonable for the scope
> > of the current PHY series, and it does help keep the macro set simple.
> > It might just be worth documenting this assumption (e.g. via a comment
> > or in the commit message).
> > 
> > Alternatively, if future PHY variants need to support different layouts,
> > this could be made more extensible by moving the status base offset into
> > the per-PHY data (similar to other register layout parameters). But I
> > don’t think that needs to block the current series.
> 
> If the register contents are generally similar but the bit positions
> and/or reg offsets differ, regmap_fields may be useful

Or platform-specific set of macros / reg accessors (as it was done in
QMP).

> 
> Konrad

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Dmitry Baryshkov]

On Thu, Mar 26, 2026 at 01:04:44AM +0000, Bryan O'Donoghue wrote:
> Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
> existing CAMSS CSI PHY init sequences are imported in order to save time
> and effort in later patches.
> 
> The following devices are supported in this drop:
> "qcom,x1e80100-csi2-phy"
> 
> In-line with other PHY drivers the process node is included in the name.
> Data-lane and clock lane positioning and polarity selection via newly
> amended struct phy_configure_opts_mipi_dphy{} is supported.
> 
> The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
> DPHY is supported.
> 
> In porting some of the logic over from camss-csiphy*.c to here its also
> possible to rationalise some of the code.
> 
> In particular use of regulator_bulk and clk_bulk as well as dropping the
> seemingly useless and unused interrupt handler.
> 
> The PHY sequences and a lot of the logic that goes with them are well
> proven in CAMSS and mature so the main thing to watch out for here is how
> to get the right sequencing of regulators, clocks and register-writes.
> 
> The register init sequence table is imported verbatim from the existing
> CAMSS csiphy driver. A follow-up series will rework the table to extract
> the repetitive per-lane pattern into a loop.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  MAINTAINERS                                        |  11 +
>  drivers/phy/qualcomm/Kconfig                       |  13 +
>  drivers/phy/qualcomm/Makefile                      |   5 +
>  drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 361 +++++++++++++++++++++
>  drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     | 298 +++++++++++++++++
>  drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |  95 ++++++
>  6 files changed, 783 insertions(+)
> 
> +/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
> +static const struct
> +mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
> +	/* Power up lanes 2ph mode */
> +	{.reg_addr = 0x1014, .reg_data = 0xd5, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +
> +	{.reg_addr = 0x0094, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x00a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0090, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0098, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0094, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0030, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0000, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0038, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x002c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0034, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x001c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0014, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x003c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0004, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0020, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0008, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> +	{.reg_addr = 0x0010, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0094, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x005c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0060, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0064, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},

I think, I've left similar feedback already. Please rework a single
structure-less table into generic and per-lane configuration. It doesn't
make sense to repeat the same configuration N times, just with the
different offset.

> +
> +	{.reg_addr = 0x0e94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0ea0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e28, .reg_data = 0x04, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e00, .reg_data = 0x80, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e0c, .reg_data = 0xff, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e38, .reg_data = 0x1f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0e08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> +	{.reg_addr = 0x0e10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> +
> +	{.reg_addr = 0x0494, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x04a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0490, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0498, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0494, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0430, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0400, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0438, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x042c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0434, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x041c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0414, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x043c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0404, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0420, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0408, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> +	{.reg_addr = 0x0410, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0494, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x045c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0460, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0464, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> +	{.reg_addr = 0x0894, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x08a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0890, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0898, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0894, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0830, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0800, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0838, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x082c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0834, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x081c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0814, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x083c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0804, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0820, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0808, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> +	{.reg_addr = 0x0810, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0894, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x085c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0860, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0864, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +
> +	{.reg_addr = 0x0c94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0ca0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c00, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c38, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> +	{.reg_addr = 0x0c10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0c94, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0c5c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0c60, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> +	{.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> +};
> +

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Bryan O'Donoghue]

On 27/03/2026 20:54, Dmitry Baryshkov wrote:
> I think, I've left similar feedback already. Please rework a single
> structure-less table into generic and per-lane configuration. It doesn't
> make sense to repeat the same configuration N times, just with the
> different offset.

If you remember you also said that was fine for a follow-on series after 
merging, and you asked for a commit log to that effect.

i.e.

"The register init sequence table is imported verbatim from the existing
CAMSS csiphy driver. A follow-up series will rework the table to extract
the repetitive per-lane pattern into a loop."

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

[Dmitry Baryshkov]

On Sat, 28 Mar 2026 at 00:11, Bryan O'Donoghue <bod@kernel.org> wrote:
>
> On 27/03/2026 20:54, Dmitry Baryshkov wrote:
> > I think, I've left similar feedback already. Please rework a single
> > structure-less table into generic and per-lane configuration. It doesn't
> > make sense to repeat the same configuration N times, just with the
> > different offset.
>
> If you remember you also said that was fine for a follow-on series after
> merging, and you asked for a commit log to that effect.
>
> i.e.
>
> "The register init sequence table is imported verbatim from the existing
> CAMSS csiphy driver. A follow-up series will rework the table to extract
> the repetitive per-lane pattern into a loop."

Ack.


-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy