* [PATCH v4 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
@ 2026-03-15 23:52 Bryan O'Donoghue
2026-03-15 23:52 ` [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema Bryan O'Donoghue
2026-03-15 23:52 ` [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
0 siblings, 2 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-15 23:52 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel,
Bryan O'Donoghue
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 | 133 ++++++++
MAINTAINERS | 11 +
drivers/phy/qualcomm/Kconfig | 13 +
drivers/phy/qualcomm/Makefile | 5 +
drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 364 +++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c | 289 ++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2.h | 101 ++++++
7 files changed, 916 insertions(+)
---
base-commit: c824345288d11e269ce41b36c105715bc2286050
change-id: 20250710-x1e-csi2-phy-f6434b651d3a
Best regards,
--
Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-15 23:52 [PATCH v4 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
@ 2026-03-15 23:52 ` Bryan O'Donoghue
2026-03-16 1:58 ` Vladimir Zapolskiy
` (2 more replies)
2026-03-15 23:52 ` [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
1 sibling, 3 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-15 23:52 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel,
Bryan O'Donoghue
Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
PHY devices.
The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++++++++++++++
1 file changed, 133 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..b83c2d65ebc6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
@@ -0,0 +1,133 @@
+# 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
+
+ clocks:
+ maxItems: 4
+
+ clock-names:
+ items:
+ - const: csiphy
+ - const: csiphy_timer
+ - const: camnoc_axi
+ - const: cpas_ahb
+
+ interrupts:
+ maxItems: 1
+
+ operating-points-v2:
+ maxItems: 1
+
+ power-domains:
+ items:
+ - description: TITAN TOP GDSC
+ - description: MXC or MXA voltage rail
+ - description: MMCX voltage rail
+
+ power-domain-names:
+ items:
+ - const: top
+ - const: mx
+ - const: mmcx
+
+ vdda-0p8-supply:
+ description: Phandle to a 0.8V 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-0p8-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.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ csiphy@ace4000 {
+ compatible = "qcom,x1e80100-csi2-phy";
+ reg = <0x0ace4000 0x2000>;
+ #phy-cells = <1>;
+
+ clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+ <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>;
+ clock-names = "csiphy",
+ "csiphy_timer",
+ "camnoc_axi",
+ "cpas_ahb";
+
+ operating-points-v2 = <&csiphy_opp_table>;
+
+ interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
+
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
+ <&rpmhpd RPMHPD_MX>,
+ <&rpmhpd RPMHPD_MMCX>;
+ power-domain-names = "top",
+ "mx",
+ "mmcx";
+
+ vdda-0p8-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>;
+ };
+ };
--
2.52.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-15 23:52 [PATCH v4 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
2026-03-15 23:52 ` [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema Bryan O'Donoghue
@ 2026-03-15 23:52 ` Bryan O'Donoghue
2026-03-16 10:12 ` Krzysztof Kozlowski
` (3 more replies)
1 sibling, 4 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-15 23:52 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel,
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. At
the moment we follow the assignment of lane positions - the bitmap of
physical input lanes to logical lane numbers as a linear list per the
existing DPHY @lanes data-member.
This is fine for us in upstream at the moment since we also map the lanes
contiguously but, our hardware can support different lane mappings so we
should in the future extend out the DPHY structure to capture the mapping.
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 | 364 +++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c | 289 ++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2.h | 101 ++++++
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..874c5c2cb01c8
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
@@ -0,0 +1,364 @@
+// 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[] = {
+ "camnoc_axi",
+ "cpas_ahb",
+ "csiphy",
+ "csiphy_timer"
+};
+
+static const char * const x1e_supplies[] = {
+ "vdda-0p8",
+ "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,
+ .generation = GEN2,
+ },
+ .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[2],
+ .timer_clk = x1e_clks[3],
+ .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..b5969ce66cd6d
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
@@ -0,0 +1,289 @@
+// 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 "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 = &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_opts);
+ if (ret)
+ return ret;
+
+ if (dphy_cfg_opts->lanes < 1 || dphy_cfg_opts->lanes > CSI2_MAX_DATA_LANES)
+ return -EINVAL;
+
+ stream_cfg->combo_mode = 0;
+ stream_cfg->link_freq = dphy_cfg_opts->hs_clk_rate;
+ stream_cfg->num_data_lanes = dphy_cfg_opts->lanes;
+
+ /*
+ * phy_configure_opts_mipi_dphy.lanes starts from zero to
+ * the maximum number of enabled lanes.
+ *
+ * TODO: add support for bitmask of enabled lanes and polarities
+ * of those lanes to the phy_configure_opts_mipi_dphy struct.
+ * For now take the polarities as zero and the position as fixed
+ * this is fine as no current upstream implementation maps otherwise.
+ */
+ for (i = 0; i < stream_cfg->num_data_lanes; i++) {
+ stream_cfg->lane_cfg.data[i].pol = 0;
+ stream_cfg->lane_cfg.data[i].pos = i;
+ }
+
+ stream_cfg->lane_cfg.clk.pol = 0;
+ stream_cfg->lane_cfg.clk.pos = 7;
+
+ 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 (int 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 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;
+ 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, of_phy_simple_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..179f535121aad
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
@@ -0,0 +1,101 @@
+/* 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 {
+ u8 combo_mode;
+ 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;
+ enum {
+ GEN1 = 0,
+ GEN1_660,
+ GEN1_670,
+ GEN2,
+ } generation;
+};
+
+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;
+
+ 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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-15 23:52 ` [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema Bryan O'Donoghue
@ 2026-03-16 1:58 ` Vladimir Zapolskiy
2026-03-16 2:45 ` Dmitry Baryshkov
2026-03-16 7:42 ` Krzysztof Kozlowski
2026-03-16 21:31 ` Vijay Kumar Tumati
2 siblings, 1 reply; 33+ messages in thread
From: Vladimir Zapolskiy @ 2026-03-16 1:58 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 3/16/26 01:52, Bryan O'Donoghue wrote:
> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
> PHY devices.
>
> The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++++++++++++++
> 1 file changed, 133 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..b83c2d65ebc6e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> @@ -0,0 +1,133 @@
> +# 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
Hence there should be a description of phy-type property, or it could be
specified in a cell on the client's side.
> + modes.
> +
> +properties:
> + compatible:
> + const: qcom,x1e80100-csi2-phy
> +
> + reg:
> + maxItems: 1
> +
> + "#phy-cells":
> + const: 1
The description is missing.
> +
> + clocks:
> + maxItems: 4
> +
> + clock-names:
> + items:
> + - const: csiphy
> + - const: csiphy_timer
> + - const: camnoc_axi
> + - const: cpas_ahb
> +
> + interrupts:
> + maxItems: 1
> +
> + operating-points-v2:
> + maxItems: 1
> +
> + power-domains:
> + items:
> + - description: TITAN TOP GDSC
> + - description: MXC or MXA voltage rail
> + - description: MMCX voltage rail
> +
> + power-domain-names:
> + items:
> + - const: top
> + - const: mx
> + - const: mmcx
> +
> + vdda-0p8-supply:
> + description: Phandle to a 0.8V regulator supply to a PHY.
On Hamoa the CSIPHY supply voltage is 0.88-0.92, so it is 0p9 rather than 0p8.
> +
> + 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-0p8-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.h>
> + #include <dt-bindings/power/qcom,rpmhpd.h>
Please sort the list of headers alphanumerically.
> +
> + csiphy@ace4000 {
> + compatible = "qcom,x1e80100-csi2-phy";
> + reg = <0x0ace4000 0x2000>;
> + #phy-cells = <1>;
> +
> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> + <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>;
> + clock-names = "csiphy",
> + "csiphy_timer",
> + "camnoc_axi",
> + "cpas_ahb";
> +
> + operating-points-v2 = <&csiphy_opp_table>;
> +
> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> + power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
> + <&rpmhpd RPMHPD_MX>,
> + <&rpmhpd RPMHPD_MMCX>;
> + power-domain-names = "top",
> + "mx",
> + "mmcx";
> +
> + vdda-0p8-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>;
> + };
> + };
>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-16 1:58 ` Vladimir Zapolskiy
@ 2026-03-16 2:45 ` Dmitry Baryshkov
2026-03-16 10:49 ` Konrad Dybcio
0 siblings, 1 reply; 33+ messages in thread
From: Dmitry Baryshkov @ 2026-03-16 2:45 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On Mon, Mar 16, 2026 at 03:58:14AM +0200, Vladimir Zapolskiy wrote:
> On 3/16/26 01:52, Bryan O'Donoghue wrote:
> > Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
> > PHY devices.
> >
> > The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++++++++++++++
> > 1 file changed, 133 insertions(+)
> >
> > +
> > + vdda-0p8-supply:
> > + description: Phandle to a 0.8V regulator supply to a PHY.
>
> On Hamoa the CSIPHY supply voltage is 0.88-0.92, so it is 0p9 rather than 0p8.
What is its name in the schematics or in the datasheet?
>
> > +
> > + vdda-1p2-supply:
> > + description: Phandle to 1.2V regulator supply to a PHY.
> > +
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-15 23:52 ` [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema Bryan O'Donoghue
2026-03-16 1:58 ` Vladimir Zapolskiy
@ 2026-03-16 7:42 ` Krzysztof Kozlowski
2026-03-16 21:31 ` Vijay Kumar Tumati
2 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-16 7:42 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On Sun, Mar 15, 2026 at 11:52:06PM +0000, Bryan O'Donoghue wrote:
> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
> PHY devices.
>
> The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++++++++++++++
> 1 file changed, 133 insertions(+)
>
<form letter>
This is a friendly reminder during the review process.
It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.
Thank you.
</form letter>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-15 23:52 ` [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
@ 2026-03-16 10:12 ` Krzysztof Kozlowski
2026-03-16 12:04 ` Bryan O'Donoghue
2026-03-18 10:15 ` Neil Armstrong
` (2 subsequent siblings)
3 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-16 10:12 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 16/03/2026 00:52, Bryan O'Donoghue wrote:
> +
> + csi2phy = devm_kzalloc(dev, sizeof(*csi2phy), GFP_KERNEL);
> + if (!csi2phy)
> + return -ENOMEM;
> +
> + csi2phy->dev = dev;
> + 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])) {
API is terrible, but it does return NULL. Look at other uses of it. Or
read review of sashiko of your patchset.
> + return dev_err_probe(dev, PTR_ERR(csi2phy->pds[i]),
> + "Failed to attach %s\n",
> + csi2phy->soc_cfg->genpd_names[i]);
> + }
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-16 2:45 ` Dmitry Baryshkov
@ 2026-03-16 10:49 ` Konrad Dybcio
2026-03-16 12:09 ` Bryan O'Donoghue
0 siblings, 1 reply; 33+ messages in thread
From: Konrad Dybcio @ 2026-03-16 10:49 UTC (permalink / raw)
To: Dmitry Baryshkov, Vladimir Zapolskiy
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 3/16/26 3:45 AM, Dmitry Baryshkov wrote:
> On Mon, Mar 16, 2026 at 03:58:14AM +0200, Vladimir Zapolskiy wrote:
>> On 3/16/26 01:52, Bryan O'Donoghue wrote:
>>> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
>>> PHY devices.
>>>
>>> The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++++++++++++++
>>> 1 file changed, 133 insertions(+)
>>>
>>> +
>>> + vdda-0p8-supply:
>>> + description: Phandle to a 0.8V regulator supply to a PHY.
>>
>> On Hamoa the CSIPHY supply voltage is 0.88-0.92, so it is 0p9 rather than 0p8.
>
> What is its name in the schematics or in the datasheet?
VDD_A_CSI_n_0P9
so vdda-0p9 seems fitting
Konrad
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-16 10:12 ` Krzysztof Kozlowski
@ 2026-03-16 12:04 ` Bryan O'Donoghue
0 siblings, 0 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-16 12:04 UTC (permalink / raw)
To: Krzysztof Kozlowski, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 16/03/2026 10:12, Krzysztof Kozlowski wrote:
>> + if (IS_ERR(csi2phy->pds[i])) {
> API is terrible, but it does return NULL.
Ah b******s... thanks missed that completely.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-16 10:49 ` Konrad Dybcio
@ 2026-03-16 12:09 ` Bryan O'Donoghue
0 siblings, 0 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-16 12:09 UTC (permalink / raw)
To: Konrad Dybcio, Dmitry Baryshkov, Vladimir Zapolskiy
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 16/03/2026 10:49, Konrad Dybcio wrote:
>>> On Hamoa the CSIPHY supply voltage is 0.88-0.92, so it is 0p9 rather than 0p8.
>> What is its name in the schematics or in the datasheet?
> VDD_A_CSI_n_0P9
>
> so vdda-0p9 seems fitting
>
> Konrad
(0.88+0.92) / 2 == 0.9
I'm happy with the average.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-15 23:52 ` [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema Bryan O'Donoghue
2026-03-16 1:58 ` Vladimir Zapolskiy
2026-03-16 7:42 ` Krzysztof Kozlowski
@ 2026-03-16 21:31 ` Vijay Kumar Tumati
2026-03-17 5:26 ` Bryan O'Donoghue
2 siblings, 1 reply; 33+ messages in thread
From: Vijay Kumar Tumati @ 2026-03-16 21:31 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
Hi Bryan,
On 3/15/2026 4:52 PM, Bryan O'Donoghue wrote:
> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
> PHY devices.
>
> The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++++++++++++++
> 1 file changed, 133 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..b83c2d65ebc6e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> @@ -0,0 +1,133 @@
> +# 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
> +
> + clocks:
> + maxItems: 4
> +
> + clock-names:
> + items:
> + - const: csiphy
> + - const: csiphy_timer
> + - const: camnoc_axi
> + - const: cpas_ahb
> +
> + interrupts:
> + maxItems: 1
> +
> + operating-points-v2:
> + maxItems: 1
> +
> + power-domains:
> + items:
> + - description: TITAN TOP GDSC
> + - description: MXC or MXA voltage rail
Would it be better to provision MXA or MXC as an additional optional
power domain? I see 'cam_cc_cphy_rx_clk_src', the parent of all CSIPHYx
clocks, need all three power domains on this chipset.
> + - description: MMCX voltage rail
> +
> + power-domain-names:
> + items:
> + - const: top
> + - const: mx
> + - const: mmcx
> +
> + vdda-0p8-supply:
> + description: Phandle to a 0.8V 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-0p8-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.h>
> + #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> + csiphy@ace4000 {
> + compatible = "qcom,x1e80100-csi2-phy";
> + reg = <0x0ace4000 0x2000>;
> + #phy-cells = <1>;
> +
> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> + <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>;
> + clock-names = "csiphy",
> + "csiphy_timer",
> + "camnoc_axi",
> + "cpas_ahb";
Although it's not a concern from my side, just want to be explicitly
sure that everyone is happy with the clock names, just to avoid any
changes later on when other modules are separated out.
> +
> + operating-points-v2 = <&csiphy_opp_table>;
> +
> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> + power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
> + <&rpmhpd RPMHPD_MX>,
> + <&rpmhpd RPMHPD_MMCX>;
> + power-domain-names = "top",
> + "mx",
> + "mmcx";
> +
> + vdda-0p8-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>;
480mhz should be svs?
> + };
> + };
>
Thanks,
Vijay.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-16 21:31 ` Vijay Kumar Tumati
@ 2026-03-17 5:26 ` Bryan O'Donoghue
2026-03-17 20:25 ` Vijay Kumar Tumati
0 siblings, 1 reply; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-17 5:26 UTC (permalink / raw)
To: Vijay Kumar Tumati, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 16/03/2026 21:31, Vijay Kumar Tumati wrote:
> Hi Bryan,
>
> On 3/15/2026 4:52 PM, Bryan O'Donoghue wrote:
>> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
>> PHY devices.
>>
>> The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++++++++++++++
>> 1 file changed, 133 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..b83c2d65ebc6e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>> @@ -0,0 +1,133 @@
>> +# 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
>> +
>> + clocks:
>> + maxItems: 4
>> +
>> + clock-names:
>> + items:
>> + - const: csiphy
>> + - const: csiphy_timer
>> + - const: camnoc_axi
>> + - const: cpas_ahb
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + operating-points-v2:
>> + maxItems: 1
>> +
>> + power-domains:
>> + items:
>> + - description: TITAN TOP GDSC
>> + - description: MXC or MXA voltage rail
> Would it be better to provision MXA or MXC as an additional optional
> power domain? I see 'cam_cc_cphy_rx_clk_src', the parent of all CSIPHYx
> clocks, need all three power domains on this chipset.
I don't think this should be optional. Have the dts point to an "mx"
power-domain and then select which one is right for a PHY MX/MXA or MXC.
Your worst case here is some future PHY which has more or fewer PDs
which is then either a special case in this file or a whole new file for
that compat.
>> + - description: MMCX voltage rail
>> +
>> + power-domain-names:
>> + items:
>> + - const: top
>> + - const: mx
>> + - const: mmcx
>> +
>> + vdda-0p8-supply:
>> + description: Phandle to a 0.8V 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-0p8-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.h>
>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>> +
>> + csiphy@ace4000 {
>> + compatible = "qcom,x1e80100-csi2-phy";
>> + reg = <0x0ace4000 0x2000>;
>> + #phy-cells = <1>;
>> +
>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
>> + <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
>> + <&camcc CAM_CC_CPAS_AHB_CLK>;
>> + clock-names = "csiphy",
>> + "csiphy_timer",
>> + "camnoc_axi",
>> + "cpas_ahb";
> Although it's not a concern from my side, just want to be explicitly
> sure that everyone is happy with the clock names, just to avoid any
> changes later on when other modules are separated out.
These are the names we already use in CAMSS so ... they're good enough
to start from.
>> +
>> + operating-points-v2 = <&csiphy_opp_table>;
>> +
>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>> +
>> + power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
>> + <&rpmhpd RPMHPD_MX>,
>> + <&rpmhpd RPMHPD_MMCX>;
>> + power-domain-names = "top",
>> + "mx",
>> + "mmcx";
>> +
>> + vdda-0p8-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>;
> 480mhz should be svs?
Yes you're right thanks for spotting.
>> + };
>> + };
>>
> Thanks,
> Vijay.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-17 5:26 ` Bryan O'Donoghue
@ 2026-03-17 20:25 ` Vijay Kumar Tumati
2026-03-23 14:22 ` Konrad Dybcio
0 siblings, 1 reply; 33+ messages in thread
From: Vijay Kumar Tumati @ 2026-03-17 20:25 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 3/16/2026 10:26 PM, Bryan O'Donoghue wrote:
> On 16/03/2026 21:31, Vijay Kumar Tumati wrote:
>> Hi Bryan,
>>
>> On 3/15/2026 4:52 PM, Bryan O'Donoghue wrote:
>>> Add a base schema initially compatible with x1e80100 to describe MIPI
>>> CSI2
>>> PHY devices.
>>>
>>> The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++
>>> ++++++++++++
>>> 1 file changed, 133 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..b83c2d65ebc6e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>> @@ -0,0 +1,133 @@
>>> +# 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
>>> +
>>> + clocks:
>>> + maxItems: 4
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: csiphy
>>> + - const: csiphy_timer
>>> + - const: camnoc_axi
>>> + - const: cpas_ahb
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + operating-points-v2:
>>> + maxItems: 1
>>> +
>>> + power-domains:
>>> + items:
>>> + - description: TITAN TOP GDSC
>>> + - description: MXC or MXA voltage rail
>> Would it be better to provision MXA or MXC as an additional optional
>> power domain? I see 'cam_cc_cphy_rx_clk_src', the parent of all CSIPHYx
>> clocks, need all three power domains on this chipset.
>
> I don't think this should be optional. Have the dts point to an "mx"
> power-domain and then select which one is right for a PHY MX/MXA or MXC.
>
> Your worst case here is some future PHY which has more or fewer PDs
> which is then either a special case in this file or a whole new file for
> that compat.
>
I think it is the case on x1e* as well, Bryan.
>>> + - description: MMCX voltage rail
>>> +
>>> + power-domain-names:
>>> + items:
>>> + - const: top
>>> + - const: mx
>>> + - const: mmcx
>>> +
>>> + vdda-0p8-supply:
>>> + description: Phandle to a 0.8V 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-0p8-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.h>
>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>> +
>>> + csiphy@ace4000 {
>>> + compatible = "qcom,x1e80100-csi2-phy";
>>> + reg = <0x0ace4000 0x2000>;
>>> + #phy-cells = <1>;
>>> +
>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
>>> + <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
>>> + <&camcc CAM_CC_CPAS_AHB_CLK>;
>>> + clock-names = "csiphy",
>>> + "csiphy_timer",
>>> + "camnoc_axi",
>>> + "cpas_ahb";
>> Although it's not a concern from my side, just want to be explicitly
>> sure that everyone is happy with the clock names, just to avoid any
>> changes later on when other modules are separated out.
>
> These are the names we already use in CAMSS so ... they're good enough
> to start from.
>
Sure, FYI: Dmitry, Konrad.
>>> +
>>> + operating-points-v2 = <&csiphy_opp_table>;
>>> +
>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>> +
>>> + power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
>>> + <&rpmhpd RPMHPD_MX>,
>>> + <&rpmhpd RPMHPD_MMCX>;
>>> + power-domain-names = "top",
>>> + "mx",
>>> + "mmcx";
>>> +
>>> + vdda-0p8-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>;
>> 480mhz should be svs?
>
> Yes you're right thanks for spotting.
>
>>> + };
>>> + };
>>>
>> Thanks,
>> Vijay.
>
Thanks,
Vijay.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-15 23:52 ` [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
2026-03-16 10:12 ` Krzysztof Kozlowski
@ 2026-03-18 10:15 ` Neil Armstrong
2026-03-18 13:17 ` Bryan O'Donoghue
2026-03-22 16:44 ` kernel test robot
2026-03-22 23:31 ` kernel test robot
3 siblings, 1 reply; 33+ messages in thread
From: Neil Armstrong @ 2026-03-18 10:15 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 3/16/26 00:52, 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. At
> the moment we follow the assignment of lane positions - the bitmap of
> physical input lanes to logical lane numbers as a linear list per the
> existing DPHY @lanes data-member.
>
> This is fine for us in upstream at the moment since we also map the lanes
> contiguously but, our hardware can support different lane mappings so we
> should in the future extend out the DPHY structure to capture the mapping.
>
> 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 | 364 +++++++++++++++++++++
> drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c | 289 ++++++++++++++++
> drivers/phy/qualcomm/phy-qcom-mipi-csi2.h | 101 ++++++
> 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..874c5c2cb01c8
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> @@ -0,0 +1,364 @@
> +// 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[] = {
> + "camnoc_axi",
> + "cpas_ahb",
> + "csiphy",
> + "csiphy_timer"
> +};
> +
> +static const char * const x1e_supplies[] = {
> + "vdda-0p8",
> + "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,
> + .generation = GEN2,
> + },
> + .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[2],
> + .timer_clk = x1e_clks[3],
> + .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..b5969ce66cd6d
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> @@ -0,0 +1,289 @@
> +// 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 "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 = &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_opts);
> + if (ret)
> + return ret;
> +
> + if (dphy_cfg_opts->lanes < 1 || dphy_cfg_opts->lanes > CSI2_MAX_DATA_LANES)
> + return -EINVAL;
> +
> + stream_cfg->combo_mode = 0;
> + stream_cfg->link_freq = dphy_cfg_opts->hs_clk_rate;
> + stream_cfg->num_data_lanes = dphy_cfg_opts->lanes;
> +
> + /*
> + * phy_configure_opts_mipi_dphy.lanes starts from zero to
> + * the maximum number of enabled lanes.
> + *
> + * TODO: add support for bitmask of enabled lanes and polarities
> + * of those lanes to the phy_configure_opts_mipi_dphy struct.
> + * For now take the polarities as zero and the position as fixed
> + * this is fine as no current upstream implementation maps otherwise.
> + */
This is wrong since you loose the lanes mapping defined in DT, which is still in CAMSS
but is a PHY property. The lanes layout is not a property of the CSI controller,
CSI controller only need to know the lanes count, and not the layout.
> + for (i = 0; i < stream_cfg->num_data_lanes; i++) {
> + stream_cfg->lane_cfg.data[i].pol = 0;
> + stream_cfg->lane_cfg.data[i].pos = i;
> + }
> +
> + stream_cfg->lane_cfg.clk.pol = 0;
> + stream_cfg->lane_cfg.clk.pos = 7;
> +
> + 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 (int 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 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;
> + 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, of_phy_simple_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..179f535121aad
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> @@ -0,0 +1,101 @@
> +/* 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 {
> + u8 combo_mode;
> + 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;
> + enum {
> + GEN1 = 0,
> + GEN1_660,
> + GEN1_670,
> + GEN2,
> + } generation;
> +};
> +
> +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;
> +
> + 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__ */
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-18 10:15 ` Neil Armstrong
@ 2026-03-18 13:17 ` Bryan O'Donoghue
2026-03-18 15:07 ` Neil Armstrong
0 siblings, 1 reply; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-18 13:17 UTC (permalink / raw)
To: Neil Armstrong, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 18/03/2026 10:15, Neil Armstrong wrote:
>> + /*
>> + * phy_configure_opts_mipi_dphy.lanes starts from zero to
>> + * the maximum number of enabled lanes.
>> + *
>> + * TODO: add support for bitmask of enabled lanes and polarities
>> + * of those lanes to the phy_configure_opts_mipi_dphy struct.
>> + * For now take the polarities as zero and the position as fixed
>> + * this is fine as no current upstream implementation maps
>> otherwise.
>> + */
>
> This is wrong since you loose the lanes mapping defined in DT, which is
> still in CAMSS
> but is a PHY property. The lanes layout is not a property of the CSI
> controller,
> CSI controller only need to know the lanes count, and not the layout.
Lane layout is a PHY concern but, the PHY API gives us
phy_configure_opts_mipi_dphy which should be extended to provide layout
and polarity. This would then be of benefit to more than just qcom/camss.
Right now none of the CAMSS users for this driver depend on any other
mapping and I propose a separate series to fix
phy_configure_opts_mipi_dphy rather than introduce data-lanes to DPHY.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-18 13:17 ` Bryan O'Donoghue
@ 2026-03-18 15:07 ` Neil Armstrong
2026-03-18 15:27 ` Dmitry Baryshkov
2026-03-18 15:47 ` Bryan O'Donoghue
0 siblings, 2 replies; 33+ messages in thread
From: Neil Armstrong @ 2026-03-18 15:07 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 3/18/26 14:17, Bryan O'Donoghue wrote:
> On 18/03/2026 10:15, Neil Armstrong wrote:
>>> + /*
>>> + * phy_configure_opts_mipi_dphy.lanes starts from zero to
>>> + * the maximum number of enabled lanes.
>>> + *
>>> + * TODO: add support for bitmask of enabled lanes and polarities
>>> + * of those lanes to the phy_configure_opts_mipi_dphy struct.
>>> + * For now take the polarities as zero and the position as fixed
>>> + * this is fine as no current upstream implementation maps otherwise.
>>> + */
>>
>> This is wrong since you loose the lanes mapping defined in DT, which is still in CAMSS
>> but is a PHY property. The lanes layout is not a property of the CSI controller,
>> CSI controller only need to know the lanes count, and not the layout.
>
> Lane layout is a PHY concern but, the PHY API gives us phy_configure_opts_mipi_dphy which should be extended to provide layout and polarity. This would then be of benefit to more than just qcom/camss.
Why ? the only concern between a controller and a PHY is the lane count to calculate the bandwidth, the actual pin layout is certainly not a controller concern.
>
> Right now none of the CAMSS users for this driver depend on any other mapping and I propose a separate series to fix phy_configure_opts_mipi_dphy rather than introduce data-lanes to DPHY.
None of the upstream users of camss.
The problem is even larger, as you replied in [1], the csiphy is still exposed as a media element from the CAMSS driver, this means this driver is not complete,
it should be a media driver entirely with eventually an internal PHY aux driver, but this would be entirely implementation specific.
Either the PHY is standalone and the PHY consumer only calls phy_open/init/configure/power_on/power_off/exit, otherwise it's not a fully standaline PHY but a composite device like here.
I propose that you write a proper media driver for the qcom csiphy, which eventually spins a PHY driver as an aux device.
Neil
>
> ---
> bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-18 15:07 ` Neil Armstrong
@ 2026-03-18 15:27 ` Dmitry Baryshkov
2026-03-19 13:08 ` Vladimir Zapolskiy
2026-03-18 15:47 ` Bryan O'Donoghue
1 sibling, 1 reply; 33+ messages in thread
From: Dmitry Baryshkov @ 2026-03-18 15:27 UTC (permalink / raw)
To: Neil Armstrong
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On Wed, Mar 18, 2026 at 04:07:39PM +0100, Neil Armstrong wrote:
> On 3/18/26 14:17, Bryan O'Donoghue wrote:
> > On 18/03/2026 10:15, Neil Armstrong wrote:
> > > > + /*
> > > > + * phy_configure_opts_mipi_dphy.lanes starts from zero to
> > > > + * the maximum number of enabled lanes.
> > > > + *
> > > > + * TODO: add support for bitmask of enabled lanes and polarities
> > > > + * of those lanes to the phy_configure_opts_mipi_dphy struct.
> > > > + * For now take the polarities as zero and the position as fixed
> > > > + * this is fine as no current upstream implementation maps otherwise.
> > > > + */
> > >
> > > This is wrong since you loose the lanes mapping defined in DT, which is still in CAMSS
> > > but is a PHY property. The lanes layout is not a property of the CSI controller,
> > > CSI controller only need to know the lanes count, and not the layout.
> >
> > Lane layout is a PHY concern but, the PHY API gives us phy_configure_opts_mipi_dphy which should be extended to provide layout and polarity. This would then be of benefit to more than just qcom/camss.
>
> Why ? the only concern between a controller and a PHY is the lane count to calculate the bandwidth, the actual pin layout is certainly not a controller concern.
I think that the DT should be providing the information about the
connection of the lanes and their number on the board. Then the CSI host
might want to limit this further for whatever reasons. But I don't think
that the properties of the lanes should be configurable between the
controller and the PHY.
>
> >
> > Right now none of the CAMSS users for this driver depend on any other mapping and I propose a separate series to fix phy_configure_opts_mipi_dphy rather than introduce data-lanes to DPHY.
>
> None of the upstream users of camss.
>
> The problem is even larger, as you replied in [1], the csiphy is still exposed as a media element from the CAMSS driver, this means this driver is not complete,
> it should be a media driver entirely with eventually an internal PHY aux driver, but this would be entirely implementation specific.
>
> Either the PHY is standalone and the PHY consumer only calls phy_open/init/configure/power_on/power_off/exit, otherwise it's not a fully standaline PHY but a composite device like here.
>
> I propose that you write a proper media driver for the qcom csiphy, which eventually spins a PHY driver as an aux device.
Why do you want a media driver? Isn't PHY driver enough?
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-18 15:07 ` Neil Armstrong
2026-03-18 15:27 ` Dmitry Baryshkov
@ 2026-03-18 15:47 ` Bryan O'Donoghue
1 sibling, 0 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-18 15:47 UTC (permalink / raw)
To: Neil Armstrong, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 18/03/2026 15:07, Neil Armstrong wrote:
> On 3/18/26 14:17, Bryan O'Donoghue wrote:
>> On 18/03/2026 10:15, Neil Armstrong wrote:
>>>> + /*
>>>> + * phy_configure_opts_mipi_dphy.lanes starts from zero to
>>>> + * the maximum number of enabled lanes.
>>>> + *
>>>> + * TODO: add support for bitmask of enabled lanes and polarities
>>>> + * of those lanes to the phy_configure_opts_mipi_dphy struct.
>>>> + * For now take the polarities as zero and the position as fixed
>>>> + * this is fine as no current upstream implementation maps otherwise.
>>>> + */
>>>
>>> This is wrong since you loose the lanes mapping defined in DT, which is still in CAMSS
>>> but is a PHY property. The lanes layout is not a property of the CSI controller,
>>> CSI controller only need to know the lanes count, and not the layout.
>>
>> Lane layout is a PHY concern but, the PHY API gives us phy_configure_opts_mipi_dphy which should be extended to provide layout and polarity. This would then be of benefit to more than just qcom/camss.
>
> Why ? the only concern between a controller and a PHY is the lane count to calculate the bandwidth, the actual pin layout is certainly not a controller concern.
Controllers already get the lane count by way of data-lanes = <x y z q>
or <x y> or <x> if we didn't do that we would need to specify the
data-lanes in the controller and again in the PHY.
>>
>> Right now none of the CAMSS users for this driver depend on any other mapping and I propose a separate series to fix phy_configure_opts_mipi_dphy rather than introduce data-lanes to DPHY.
>
> None of the upstream users of camss.
No, we are establishing from x1 use of standard drivers/phy. New users
will do it this way. The posted dtsi for the laptops can use the linear
lane layout and default polarities.
In a follow on series we can extend phy_configure_opts_mipi_dphy to
parse data-lanes = <> into count and mask, to the benefit of any user of
phy_configure_opts_mipi_dphy.
Since that will touch more then qcom specific stuff and will touch at
least two subsystems, that should be its own separate series.
> The problem is even larger, as you replied in [1], the csiphy is still exposed as a media element from the CAMSS driver, this means this driver is not complete,
> it should be a media driver entirely with eventually an internal PHY aux driver, but this would be entirely implementation specific.
>
> Either the PHY is standalone and the PHY consumer only calls phy_open/init/configure/power_on/power_off/exit, otherwise it's not a fully standaline PHY but a composite device like here.
This is not a composite device any more than the existing upstream
implementations which follow the same model:
- Cadence CSI2RX + Cadence DPHY (TI J721E/AM62A)
- Rockchip rkisp1 + phy-rockchip-inno-csidphy
Both use phys = <&phandle>, the media driver manages V4L2 endpoints
and lane counts, the PHY driver handles the electrical layer via
phy_configure().
To this list we will add qcom camss, there's nothing exotic being proposed.
> I propose that you write a proper media driver for the qcom csiphy, which eventually spins a PHY driver as an aux device.
None of these SoC D-PHYs are written as V4L2 media drivers that spawn
auxiliary devices. They all use the phys = <&phandle> model. The media
driver manages the V4L2 endpoints and lane counts, passing the
configuration down via phy_configure_opts_mipi_dphy.
I just don't see what is so special about CAMSS that it needs to have
its own special PHY implementation. drivers/phy the standard API and
specification of data-lanes etc in the controller seems pretty "bog
standard".
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-18 15:27 ` Dmitry Baryshkov
@ 2026-03-19 13:08 ` Vladimir Zapolskiy
2026-03-19 13:17 ` Bryan O'Donoghue
0 siblings, 1 reply; 33+ messages in thread
From: Vladimir Zapolskiy @ 2026-03-19 13:08 UTC (permalink / raw)
To: Dmitry Baryshkov, Neil Armstrong
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 3/18/26 17:27, Dmitry Baryshkov wrote:
> On Wed, Mar 18, 2026 at 04:07:39PM +0100, Neil Armstrong wrote:
>> On 3/18/26 14:17, Bryan O'Donoghue wrote:
>>> On 18/03/2026 10:15, Neil Armstrong wrote:
>>>>> + /*
>>>>> + * phy_configure_opts_mipi_dphy.lanes starts from zero to
>>>>> + * the maximum number of enabled lanes.
>>>>> + *
>>>>> + * TODO: add support for bitmask of enabled lanes and polarities
>>>>> + * of those lanes to the phy_configure_opts_mipi_dphy struct.
>>>>> + * For now take the polarities as zero and the position as fixed
>>>>> + * this is fine as no current upstream implementation maps otherwise.
>>>>> + */
>>>>
>>>> This is wrong since you loose the lanes mapping defined in DT, which is still in CAMSS
>>>> but is a PHY property. The lanes layout is not a property of the CSI controller,
>>>> CSI controller only need to know the lanes count, and not the layout.
>>>
>>> Lane layout is a PHY concern but, the PHY API gives us phy_configure_opts_mipi_dphy which should be extended to provide layout and polarity. This would then be of benefit to more than just qcom/camss.
>>
>> Why ? the only concern between a controller and a PHY is the lane count to calculate the bandwidth, the actual pin layout is certainly not a controller concern.
>
> I think that the DT should be providing the information about the
> connection of the lanes and their number on the board. Then the CSI host
> might want to limit this further for whatever reasons. But I don't think
> that the properties of the lanes should be configurable between the
> controller and the PHY.
>
>>
>>>
>>> Right now none of the CAMSS users for this driver depend on any other mapping and I propose a separate series to fix phy_configure_opts_mipi_dphy rather than introduce data-lanes to DPHY.
>>
>> None of the upstream users of camss.
>>
>> The problem is even larger, as you replied in [1], the csiphy is still exposed as a media element from the CAMSS driver, this means this driver is not complete,
>> it should be a media driver entirely with eventually an internal PHY aux driver, but this would be entirely implementation specific.
>>
>> Either the PHY is standalone and the PHY consumer only calls phy_open/init/configure/power_on/power_off/exit, otherwise it's not a fully standaline PHY but a composite device like here.
>>
>> I propose that you write a proper media driver for the qcom csiphy, which eventually spins a PHY driver as an aux device.
>
> Why do you want a media driver? Isn't PHY driver enough?
>
As for today CAMSS CSIPHY are already media devices, and a user applies media
specific properties to them, for instance media bus format, resolution etc.
Technically this might be removed from CAMSS, but if so, then it should be
done before this new PHY driver model is applied.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 13:08 ` Vladimir Zapolskiy
@ 2026-03-19 13:17 ` Bryan O'Donoghue
2026-03-19 14:05 ` Neil Armstrong
2026-03-19 14:56 ` Vladimir Zapolskiy
0 siblings, 2 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-19 13:17 UTC (permalink / raw)
To: Vladimir Zapolskiy, Dmitry Baryshkov, Neil Armstrong
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 19/03/2026 13:08, Vladimir Zapolskiy wrote:
>> Why do you want a media driver? Isn't PHY driver enough?
>>
> As for today CAMSS CSIPHY are already media devices, and a user applies media
> specific properties to them, for instance media bus format, resolution etc.
> Technically this might be removed from CAMSS, but if so, then it should be
> done before this new PHY driver model is applied.
>
> --
> Best wishes,
There's no reason to remove that from CAMSS - it would be an ABI break
in user-space anyway.
The media entity in CAMSS msm_csiphyX handles format negotiation and
pipeline routing. The PHY driver handles electrical configuration. They
don't conflict and there multiple cited examples of this upstream already.
--
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 13:17 ` Bryan O'Donoghue
@ 2026-03-19 14:05 ` Neil Armstrong
2026-03-19 15:06 ` Bryan O'Donoghue
2026-03-19 14:56 ` Vladimir Zapolskiy
1 sibling, 1 reply; 33+ messages in thread
From: Neil Armstrong @ 2026-03-19 14:05 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Dmitry Baryshkov
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 3/19/26 14:17, Bryan O'Donoghue wrote:
> On 19/03/2026 13:08, Vladimir Zapolskiy wrote:
>>> Why do you want a media driver? Isn't PHY driver enough?
>>>
>> As for today CAMSS CSIPHY are already media devices, and a user applies media
>> specific properties to them, for instance media bus format, resolution etc.
>> Technically this might be removed from CAMSS, but if so, then it should be
>> done before this new PHY driver model is applied.
>>
>> --
>> Best wishes,
>
> There's no reason to remove that from CAMSS - it would be an ABI break in user-space anyway.
>
> The media entity in CAMSS msm_csiphyX handles format negotiation and pipeline routing. The PHY driver handles electrical configuration. They don't conflict and there multiple cited examples of this upstream already.
If csiphy component was only handling electrical configuration, the only code handling csiphy would be phy API calls, not be part of the pipeline configuration. Today, it's a media element
The whole CAMSS architecture is wrong, it should be modular, each hardware module should be an independent driver and all be connected via port/endpoint and configured with the media controller API.
If you _really_ want to move the "electrical configuration" part of the CSPIPHY out of camss frankendriver, fine, then first just create an internal PHY device as an aux device, then continue migrating _all_ CAMSS components into independent driver modules, then in the end re-architecture the whole DT description by adding a node per component with a proper port/endpoint representation to be configured via the media controller API.
Neil
>
> --
> bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 13:17 ` Bryan O'Donoghue
2026-03-19 14:05 ` Neil Armstrong
@ 2026-03-19 14:56 ` Vladimir Zapolskiy
2026-03-19 15:18 ` Bryan O'Donoghue
1 sibling, 1 reply; 33+ messages in thread
From: Vladimir Zapolskiy @ 2026-03-19 14:56 UTC (permalink / raw)
To: Bryan O'Donoghue, Dmitry Baryshkov, Neil Armstrong
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 3/19/26 15:17, Bryan O'Donoghue wrote:
> On 19/03/2026 13:08, Vladimir Zapolskiy wrote:
>>> Why do you want a media driver? Isn't PHY driver enough?
>>>
>> As for today CAMSS CSIPHY are already media devices, and a user applies media
>> specific properties to them, for instance media bus format, resolution etc.
>> Technically this might be removed from CAMSS, but if so, then it should be
>> done before this new PHY driver model is applied.
>>
>> --
>> Best wishes,
>
> There's no reason to remove that from CAMSS - it would be an ABI break
> in user-space anyway.
If technically CAMSS CSIPHY could be excluded from the list of CAMSS media
subdevices, then for the sake of simplification it should be done for all
supported platforms in advance, such a change will be independent from this
particular phy series, and vice versa, this CAMSS only driver change will
prepare a ground for media-less CAMSS CSIPHY device drivers, hence it shall
precede this particular CAMSS CSIPHY series.
For backward compatibility with userspace a noop stub will be good enough,
it's not an issue at all.
> The media entity in CAMSS msm_csiphyX handles format negotiation and
> pipeline routing. The PHY driver handles electrical configuration. They
> don't conflict and there multiple cited examples of this upstream already.
>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 14:05 ` Neil Armstrong
@ 2026-03-19 15:06 ` Bryan O'Donoghue
0 siblings, 0 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-19 15:06 UTC (permalink / raw)
To: Neil Armstrong, Vladimir Zapolskiy, Dmitry Baryshkov
Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel
On 19/03/2026 14:05, Neil Armstrong wrote:
>> There's no reason to remove that from CAMSS - it would be an ABI break in user-space anyway.
>>
>> The media entity in CAMSS msm_csiphyX handles format negotiation and pipeline routing. The PHY driver handles electrical configuration. They don't conflict and there multiple cited examples of this upstream already.
> If csiphy component was only handling electrical configuration, the only code handling csiphy would be phy API calls, not be part of the pipeline configuration. Today, it's a media element
>
> The whole CAMSS architecture is wrong, it should be modular, each hardware module should be an independent driver and all be connected via port/endpoint and configured with the media controller API.
>
> If you_really_ want to move the "electrical configuration" part of the CSPIPHY out of camss frankendriver, fine, then first just create an internal PHY device as an aux device, then continue migrating_all_ CAMSS components into independent driver modules, then in the end re-architecture the whole DT description by adding a node per component with a proper port/endpoint representation to be configured via the media controller API.
>
> Neil
Re-architecting the whole driver without breaking legacy code is well
out of scope.
I'd note making a standalone CSIPHY driver 100% fits in with such a
goal. I don't think I've seen one good reason why CAMSS needs an aux
device and can't follow established upstream paradigms for Cadence and
Rockchip.
Anyway I take the feedback on polarities and will in v5 of this patch
address.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 14:56 ` Vladimir Zapolskiy
@ 2026-03-19 15:18 ` Bryan O'Donoghue
2026-03-19 16:08 ` Neil Armstrong
2026-03-20 0:37 ` Vladimir Zapolskiy
0 siblings, 2 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-19 15:18 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Dmitry Baryshkov,
Neil Armstrong
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
On 19/03/2026 14:56, Vladimir Zapolskiy wrote:
>> There's no reason to remove that from CAMSS - it would be an ABI break
>> in user-space anyway.
>
> If technically CAMSS CSIPHY could be excluded from the list of CAMSS media
> subdevices, then for the sake of simplification it should be done for all
> supported platforms in advance, such a change will be independent from this
> particular phy series, and vice versa, this CAMSS only driver change will
> prepare a ground for media-less CAMSS CSIPHY device drivers, hence it shall
> precede this particular CAMSS CSIPHY series.
>
> For backward compatibility with userspace a noop stub will be good enough,
> it's not an issue at all.
The standalone PHY driver doesn't require removing the CSIPHY media
entity from CAMSS. They serve different purposes and coexist - its
important to have a NOP from user-space perspective for legacy and
indeed for new implementations.
How the PHY gets represented in the kernel is of zero interest to
user-sapce.
That said, stubbing out the media entity is independent work that can
happen in any order and IMO is a separate debate. Whether or not CSIPHY
init sequences live inside of a monolithic CAMSS driver or live inside
off a discrete csiphy driver is not related to the media graph.
Happy to have that debate - and if indicated, carefully apply patches
separately.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 15:18 ` Bryan O'Donoghue
@ 2026-03-19 16:08 ` Neil Armstrong
2026-03-19 16:56 ` Bryan O'Donoghue
2026-03-20 0:37 ` Vladimir Zapolskiy
1 sibling, 1 reply; 33+ messages in thread
From: Neil Armstrong @ 2026-03-19 16:08 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Bryan O'Donoghue,
Dmitry Baryshkov
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
On 3/19/26 16:18, Bryan O'Donoghue wrote:
> On 19/03/2026 14:56, Vladimir Zapolskiy wrote:
>>> There's no reason to remove that from CAMSS - it would be an ABI break
>>> in user-space anyway.
>>
>> If technically CAMSS CSIPHY could be excluded from the list of CAMSS media
>> subdevices, then for the sake of simplification it should be done for all
>> supported platforms in advance, such a change will be independent from this
>> particular phy series, and vice versa, this CAMSS only driver change will
>> prepare a ground for media-less CAMSS CSIPHY device drivers, hence it shall
>> precede this particular CAMSS CSIPHY series.
>>
>> For backward compatibility with userspace a noop stub will be good enough,
>> it's not an issue at all.
>
> The standalone PHY driver doesn't require removing the CSIPHY media
> entity from CAMSS. They serve different purposes and coexist - its important to have a NOP from user-space perspective for legacy and indeed for new implementations.
>
> How the PHY gets represented in the kernel is of zero interest to user-sapce.
>
> That said, stubbing out the media entity is independent work that can happen in any order and IMO is a separate debate. Whether or not CSIPHY init sequences live inside of a monolithic CAMSS driver or live inside off a discrete csiphy driver is not related to the media graph.
>
> Happy to have that debate - and if indicated, carefully apply patches separately.
So what does this actually solves ?
Neil
>
> ---
> bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 16:08 ` Neil Armstrong
@ 2026-03-19 16:56 ` Bryan O'Donoghue
2026-03-19 17:39 ` Neil Armstrong
0 siblings, 1 reply; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-19 16:56 UTC (permalink / raw)
To: Neil Armstrong, Vladimir Zapolskiy, Bryan O'Donoghue,
Dmitry Baryshkov
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
On 19/03/2026 16:08, Neil Armstrong wrote:
> On 3/19/26 16:18, Bryan O'Donoghue wrote:
>> On 19/03/2026 14:56, Vladimir Zapolskiy wrote:
>>>> There's no reason to remove that from CAMSS - it would be an ABI break
>>>> in user-space anyway.
>>>
>>> If technically CAMSS CSIPHY could be excluded from the list of CAMSS
>>> media
>>> subdevices, then for the sake of simplification it should be done for
>>> all
>>> supported platforms in advance, such a change will be independent
>>> from this
>>> particular phy series, and vice versa, this CAMSS only driver change
>>> will
>>> prepare a ground for media-less CAMSS CSIPHY device drivers, hence it
>>> shall
>>> precede this particular CAMSS CSIPHY series.
>>>
>>> For backward compatibility with userspace a noop stub will be good
>>> enough,
>>> it's not an issue at all.
>>
>> The standalone PHY driver doesn't require removing the CSIPHY media
>> entity from CAMSS. They serve different purposes and coexist - its
>> important to have a NOP from user-space perspective for legacy and
>> indeed for new implementations.
>>
>> How the PHY gets represented in the kernel is of zero interest to
>> user-sapce.
>>
>> That said, stubbing out the media entity is independent work that can
>> happen in any order and IMO is a separate debate. Whether or not
>> CSIPHY init sequences live inside of a monolithic CAMSS driver or live
>> inside off a discrete csiphy driver is not related to the media graph.
>>
>> Happy to have that debate - and if indicated, carefully apply patches
>> separately.
>
> So what does this actually solves ?
>
> Neil
Per-PHY voltage rails, per-PHY power domains and per-PHY OPP scaling.
Using the PHY API instead of rolling our own, as well as separate nodes
in the DT.
We've been getting away with power-domains, opp scaling etc by sheer
luck. The feedback from the list alone now addressed in this driver
makes the conversion worthwhile.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 16:56 ` Bryan O'Donoghue
@ 2026-03-19 17:39 ` Neil Armstrong
0 siblings, 0 replies; 33+ messages in thread
From: Neil Armstrong @ 2026-03-19 17:39 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Dmitry Baryshkov
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
On 3/19/26 17:56, Bryan O'Donoghue wrote:
> On 19/03/2026 16:08, Neil Armstrong wrote:
>> On 3/19/26 16:18, Bryan O'Donoghue wrote:
>>> On 19/03/2026 14:56, Vladimir Zapolskiy wrote:
>>>>> There's no reason to remove that from CAMSS - it would be an ABI break
>>>>> in user-space anyway.
>>>>
>>>> If technically CAMSS CSIPHY could be excluded from the list of CAMSS media
>>>> subdevices, then for the sake of simplification it should be done for all
>>>> supported platforms in advance, such a change will be independent from this
>>>> particular phy series, and vice versa, this CAMSS only driver change will
>>>> prepare a ground for media-less CAMSS CSIPHY device drivers, hence it shall
>>>> precede this particular CAMSS CSIPHY series.
>>>>
>>>> For backward compatibility with userspace a noop stub will be good enough,
>>>> it's not an issue at all.
>>>
>>> The standalone PHY driver doesn't require removing the CSIPHY media
>>> entity from CAMSS. They serve different purposes and coexist - its important to have a NOP from user-space perspective for legacy and indeed for new implementations.
>>>
>>> How the PHY gets represented in the kernel is of zero interest to user-sapce.
>>>
>>> That said, stubbing out the media entity is independent work that can happen in any order and IMO is a separate debate. Whether or not CSIPHY init sequences live inside of a monolithic CAMSS driver or live inside off a discrete csiphy driver is not related to the media graph.
>>>
>>> Happy to have that debate - and if indicated, carefully apply patches separately.
>>
>> So what does this actually solves ?
>>
>> Neil
> Per-PHY voltage rails, per-PHY power domains and per-PHY OPP scaling.
>
> Using the PHY API instead of rolling our own, as well as separate nodes in the DT.
>
> We've been getting away with power-domains, opp scaling etc by sheer luck. The feedback from the list alone now addressed in this driver makes the conversion worthwhile.
The PHY API doesn't solve that, having proper nodes solves that, you could add a separate csiphy node, add a port/endpoint between camss and the csiphy and attach a camss aux driver to the node, and it would have the same effect with little code change.
And this could be done for all the CAMSS hardware elements incrementally, and if you wish the move the electrical phy part under the phy API then you just spin a PHY aux driver controlled by the csiphy media element.
I understand you find it simpler to use the phys property in camss, but it has plenty of drawbacks like not be able to describe data link properties specific to the CSIPHY properties or easily describe new hardware layouts without having a fixed association table between phy-names and whatever CAMSS media elements interconnections.
My question would be that if we were to completely split out the CAMSS into several separate nodes linked with port/endpoint graph, to which hardware element the phys would be associated to ? is there a fixed connection between a CSID and a CSIPHY ? is seems the CSID gen2 & gen3 can actually connect to different CSIPHY meaning a CSIPHY is a not simple electrical PHY but can be dynamically connected to different consumers.
There's no way we can handle that with the PHY API.
Neil
>
> ---
> bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-19 15:18 ` Bryan O'Donoghue
2026-03-19 16:08 ` Neil Armstrong
@ 2026-03-20 0:37 ` Vladimir Zapolskiy
2026-03-20 9:56 ` Bryan O'Donoghue
1 sibling, 1 reply; 33+ messages in thread
From: Vladimir Zapolskiy @ 2026-03-20 0:37 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Dmitry Baryshkov,
Neil Armstrong
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
On 3/19/26 17:18, Bryan O'Donoghue wrote:
> On 19/03/2026 14:56, Vladimir Zapolskiy wrote:
>>> There's no reason to remove that from CAMSS - it would be an ABI break
>>> in user-space anyway.
>>
>> If technically CAMSS CSIPHY could be excluded from the list of CAMSS media
>> subdevices, then for the sake of simplification it should be done for all
>> supported platforms in advance, such a change will be independent from this
>> particular phy series, and vice versa, this CAMSS only driver change will
>> prepare a ground for media-less CAMSS CSIPHY device drivers, hence it shall
>> precede this particular CAMSS CSIPHY series.
>>
>> For backward compatibility with userspace a noop stub will be good enough,
>> it's not an issue at all.
>
> The standalone PHY driver doesn't require removing the CSIPHY media
> entity from CAMSS. They serve different purposes and coexist - its
> important to have a NOP from user-space perspective for legacy and
> indeed for new implementations.
>
There should be no two CAMSS CSIPHY device (or subdevice) drivers, where
one chop of CAMSS CSIPHY device driver remains to sit under media, and
another one is under phy subsystem, since it's a further degradation from
the current already pretty awful state of the CAMSS driver, but at least
CSIPHY is not scattered over different subsystems today.
It might be fine to move device driver parts related to CAMSS CSIPHY
driver from media subsystem to phy subsystem, however if only a partial
transition is planned, and CSIPHY device support is split into two device
(sub-)drivers, then it merely exposes a quite severe design flaw.
It looks like it's still undecided, if CAMSS CSIPHY IP is a phy or media
device, it can not be both at the same time.
> How the PHY gets represented in the kernel is of zero interest to
> user-sapce.
>
> That said, stubbing out the media entity is independent work that can
> happen in any order and IMO is a separate debate. Whether or not CSIPHY
> init sequences live inside of a monolithic CAMSS driver or live inside
> off a discrete csiphy driver is not related to the media graph.
>
> Happy to have that debate - and if indicated, carefully apply patches
> separately.
>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-20 0:37 ` Vladimir Zapolskiy
@ 2026-03-20 9:56 ` Bryan O'Donoghue
0 siblings, 0 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-20 9:56 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Dmitry Baryshkov,
Neil Armstrong
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-phy,
linux-media, devicetree, linux-kernel
On 20/03/2026 00:37, Vladimir Zapolskiy wrote:
> On 3/19/26 17:18, Bryan O'Donoghue wrote:
>> On 19/03/2026 14:56, Vladimir Zapolskiy wrote:
>>>> There's no reason to remove that from CAMSS - it would be an ABI break
>>>> in user-space anyway.
>>> If technically CAMSS CSIPHY could be excluded from the list of CAMSS media
>>> subdevices, then for the sake of simplification it should be done for all
>>> supported platforms in advance, such a change will be independent from this
>>> particular phy series, and vice versa, this CAMSS only driver change will
>>> prepare a ground for media-less CAMSS CSIPHY device drivers, hence it shall
>>> precede this particular CAMSS CSIPHY series.
>>>
>>> For backward compatibility with userspace a noop stub will be good enough,
>>> it's not an issue at all.
>> The standalone PHY driver doesn't require removing the CSIPHY media
>> entity from CAMSS. They serve different purposes and coexist - its
>> important to have a NOP from user-space perspective for legacy and
>> indeed for new implementations.
>>
> There should be no two CAMSS CSIPHY device (or subdevice) drivers, where
> one chop of CAMSS CSIPHY device driver remains to sit under media, and
> another one is under phy subsystem, since it's a further degradation from
> the current already pretty awful state of the CAMSS driver, but at least
> CSIPHY is not scattered over different subsystems today.
>
> It might be fine to move device driver parts related to CAMSS CSIPHY
> driver from media subsystem to phy subsystem, however if only a partial
> transition is planned, and CSIPHY device support is split into two device
> (sub-)drivers, then it merely exposes a quite severe design flaw.
It's not two drivers for the same device. It's two layers:
- The media entity in CAMSS is a pipeline routing abstraction.
It validates formats and connects pads. It does not program
CSIPHY hardware directly.
- The PHY driver programs the hardware — registers, clocks,
regulators, power domains.
This is the same layering as rkisp1 (media pipeline) +
phy-rockchip-inno-csidphy (electrical config). The ISP's media
entities and the standalone PHY coexist in different subsystems
without conflict.
No CSIPHY functionality is duplicated across subsystems. This segmenting
of concepts and functionality isn't even unique. We see this constantly
with USB (where the MAC lives in drivers/usb/ and the electricals in
drivers/phy/) and Display/DSI.
> It looks like it's still undecided, if CAMSS CSIPHY IP is a phy or media
> device, it can not be both at the same time.
We are maintaining the existing user-space setup which presents a
msm_csiphyX v4l2-media device, whilst moving the PHY component out into
drivers/phy where it belongs - completely transparently to user-space.
So we already have a dependency in user-space which needs to be maintained.
The concrete hardware benefits, as I alluded to Neil, are:
- Per-PHY voltage rails
- Per-PHY power domains
- Per-PHY operating points (OPPs)
Those are real-world hardware fixes which "just work" as a result of
dedicated device nodes and a driver specifically written to relieve the
technical debt we've been accruing with the existing model, for years.
Having been educated - brought to the realisation on OPPS, Power-domains
and yes voltage rails it is amazing to me this whole legacy system
hasn't exploded before now but, its certainly way past time to fix it.
We should proceed with moving init sequences, voltages, PDs and OPPs to
drivers/phy.
I'm happy to have a debate about the status of the PHY media device
after, however I have to say I'm skeptical about removing the media
device - something we could do whether the init sequences live in
drivers/media/qcom/camss or in drivers/phy BTW - I'm skeptical about
removing that v4l2 node specifically because we've had it in user-space
for a long time and I do therefore think that constitutes an ABI.
As I've said already though `rm -rf /dev/v4l2-subdev::msm_csiphy*" is
basically an entirely separate debate.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-15 23:52 ` [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
2026-03-16 10:12 ` Krzysztof Kozlowski
2026-03-18 10:15 ` Neil Armstrong
@ 2026-03-22 16:44 ` kernel test robot
2026-03-22 23:31 ` kernel test robot
3 siblings, 0 replies; 33+ messages in thread
From: kernel test robot @ 2026-03-22 16:44 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: oe-kbuild-all, Bryan O'Donoghue, Vladimir Zapolskiy,
linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
Hi Bryan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on c824345288d11e269ce41b36c105715bc2286050]
url: https://github.com/intel-lab-lkp/linux/commits/Bryan-O-Donoghue/dt-bindings-phy-qcom-Add-CSI2-C-PHY-DPHY-schema/20260316-081353
base: c824345288d11e269ce41b36c105715bc2286050
patch link: https://lore.kernel.org/r/20260315-x1e-csi2-phy-v4-2-90c09203888d%40linaro.org
patch subject: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20260323/202603230048.ZxbvVL5L-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260323/202603230048.ZxbvVL5L-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603230048.ZxbvVL5L-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c: In function 'phy_qcom_mipi_csi2_power_off':
>> drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c:153:13: warning: unused variable 'i' [-Wunused-variable]
153 | int i;
| ^
vim +/i +153 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
149
150 static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
151 {
152 struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> 153 int i;
154
155 for (int i = 0; i < csi2phy->num_pds; i++)
156 dev_pm_genpd_set_performance_state(csi2phy->pds[i], 0);
157
158 clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
159 csi2phy->clks);
160 regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
161 csi2phy->supplies);
162
163 return 0;
164 }
165
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
2026-03-15 23:52 ` [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
` (2 preceding siblings ...)
2026-03-22 16:44 ` kernel test robot
@ 2026-03-22 23:31 ` kernel test robot
3 siblings, 0 replies; 33+ messages in thread
From: kernel test robot @ 2026-03-22 23:31 UTC (permalink / raw)
To: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: llvm, oe-kbuild-all, Bryan O'Donoghue, Vladimir Zapolskiy,
linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
Hi Bryan,
kernel test robot noticed the following build errors:
[auto build test ERROR on c824345288d11e269ce41b36c105715bc2286050]
url: https://github.com/intel-lab-lkp/linux/commits/Bryan-O-Donoghue/dt-bindings-phy-qcom-Add-CSI2-C-PHY-DPHY-schema/20260316-081353
base: c824345288d11e269ce41b36c105715bc2286050
patch link: https://lore.kernel.org/r/20260315-x1e-csi2-phy-v4-2-90c09203888d%40linaro.org
patch subject: [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20260323/202603230713.5uOUC5EI-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260323/202603230713.5uOUC5EI-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603230713.5uOUC5EI-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c:360:13: error: initializer element is not a compile-time constant
.opp_clk = x1e_clks[2],
^~~~~~~~~~~
1 error generated.
vim +360 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
347
348 const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
349 .ops = &phy_qcom_mipi_csi2_ops_3ph_1_0,
350 .reg_info = {
351 .init_seq = lane_regs_x1e80100,
352 .lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
353 .common_regs_offset = 0x1000,
354 .generation = GEN2,
355 },
356 .supply_names = (const char **)x1e_supplies,
357 .num_supplies = ARRAY_SIZE(x1e_supplies),
358 .clk_names = (const char **)x1e_clks,
359 .num_clk = ARRAY_SIZE(x1e_clks),
> 360 .opp_clk = x1e_clks[2],
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-17 20:25 ` Vijay Kumar Tumati
@ 2026-03-23 14:22 ` Konrad Dybcio
2026-03-23 14:29 ` Bryan O'Donoghue
0 siblings, 1 reply; 33+ messages in thread
From: Konrad Dybcio @ 2026-03-23 14:22 UTC (permalink / raw)
To: Vijay Kumar Tumati, Bryan O'Donoghue, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 3/17/26 9:25 PM, Vijay Kumar Tumati wrote:
>
>
> On 3/16/2026 10:26 PM, Bryan O'Donoghue wrote:
>> On 16/03/2026 21:31, Vijay Kumar Tumati wrote:
>>> Hi Bryan,
>>>
>>> On 3/15/2026 4:52 PM, Bryan O'Donoghue wrote:
>>>> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
>>>> PHY devices.
>>>>
>>>> The hardware can support both C-PHY and D-PHY modes. 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 | 133 +++++++++ ++++++++++++
>>>> 1 file changed, 133 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..b83c2d65ebc6e
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>> @@ -0,0 +1,133 @@
>>>> +# 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
>>>> +
>>>> + clocks:
>>>> + maxItems: 4
>>>> +
>>>> + clock-names:
>>>> + items:
>>>> + - const: csiphy
>>>> + - const: csiphy_timer
>>>> + - const: camnoc_axi
>>>> + - const: cpas_ahb
>>>> +
>>>> + interrupts:
>>>> + maxItems: 1
>>>> +
>>>> + operating-points-v2:
>>>> + maxItems: 1
>>>> +
>>>> + power-domains:
>>>> + items:
>>>> + - description: TITAN TOP GDSC
>>>> + - description: MXC or MXA voltage rail
>>> Would it be better to provision MXA or MXC as an additional optional
>>> power domain? I see 'cam_cc_cphy_rx_clk_src', the parent of all CSIPHYx
>>> clocks, need all three power domains on this chipset.
>>
>> I don't think this should be optional. Have the dts point to an "mx" power-domain and then select which one is right for a PHY MX/MXA or MXC.
>>
>> Your worst case here is some future PHY which has more or fewer PDs which is then either a special case in this file or a whole new file for that compat.
>>
> I think it is the case on x1e* as well, Bryan.
>>>> + - description: MMCX voltage rail
>>>> +
>>>> + power-domain-names:
>>>> + items:
>>>> + - const: top
>>>> + - const: mx
>>>> + - const: mmcx
>>>> +
>>>> + vdda-0p8-supply:
>>>> + description: Phandle to a 0.8V 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-0p8-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.h>
>>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>>> +
>>>> + csiphy@ace4000 {
>>>> + compatible = "qcom,x1e80100-csi2-phy";
>>>> + reg = <0x0ace4000 0x2000>;
>>>> + #phy-cells = <1>;
>>>> +
>>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
>>>> + <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
>>>> + <&camcc CAM_CC_CPAS_AHB_CLK>;
>>>> + clock-names = "csiphy",
>>>> + "csiphy_timer",
>>>> + "camnoc_axi",
>>>> + "cpas_ahb";
>>> Although it's not a concern from my side, just want to be explicitly
>>> sure that everyone is happy with the clock names, just to avoid any
>>> changes later on when other modules are separated out.
>>
>> These are the names we already use in CAMSS so ... they're good enough to start from.
>>
> Sure, FYI: Dmitry, Konrad.
I'll admit I haven't yet read up on all of the background discussions that you
guys had on LKML, but *if* we're going to put the PHY under camss, the GDSC and
CPAS_AHB/CAMNOC_AXI_RT references should be unnecessary, given they're not
related strictly to this PHY itself, rather it sitting in a specific corner of
the subsystem that needs them to be active (see related:
https://lore.kernel.org/linux-arm-msm/cb2430f2-8601-4c72-af6b-10f1ff16c188@oss.qualcomm.com/
)
For the other names, I *think* we won't need to rely on them (i.e. only operate
the resources through PHY APIs from the V4L2 driver) and can come up with new
ones. And hence I think we can turn "csiphy" to "core" and "csiphy_timer" to
"timer" (because we really don't need to repeat the csiphy_ prefix)
Konrad
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
2026-03-23 14:22 ` Konrad Dybcio
@ 2026-03-23 14:29 ` Bryan O'Donoghue
0 siblings, 0 replies; 33+ messages in thread
From: Bryan O'Donoghue @ 2026-03-23 14:29 UTC (permalink / raw)
To: Konrad Dybcio, Vijay Kumar Tumati, Bryan O'Donoghue,
Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
devicetree, linux-kernel
On 23/03/2026 14:22, Konrad Dybcio wrote:
>> Sure, FYI: Dmitry, Konrad.
> I'll admit I haven't yet read up on all of the background discussions that you
> guys had on LKML, but*if* we're going to put the PHY under camss, the GDSC and
> CPAS_AHB/CAMNOC_AXI_RT references should be unnecessary, given they're not
> related strictly to this PHY itself, rather it sitting in a specific corner of
> the subsystem that needs them to be active (see related:
> https://lore.kernel.org/linux-arm-msm/cb2430f2-8601-4c72-
> af6b-10f1ff16c188@oss.qualcomm.com/
> )
That's fair comment with the PHYs inside of the CAMSS block. Obviously
if its outside of the block we need the full gamut of clocks defined.
..
Yeah I think I'm happy enough to drop these predicated on sub-nodes.
> For the other names, I*think* we won't need to rely on them (i.e. only operate
> the resources through PHY APIs from the V4L2 driver) and can come up with new
> ones. And hence I think we can turn "csiphy" to "core" and "csiphy_timer" to
> "timer" (because we really don't need to repeat the csiphy_ prefix)
Works for me.
---
bod
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2026-03-23 14:30 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-15 23:52 [PATCH v4 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
2026-03-15 23:52 ` [PATCH v4 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema Bryan O'Donoghue
2026-03-16 1:58 ` Vladimir Zapolskiy
2026-03-16 2:45 ` Dmitry Baryshkov
2026-03-16 10:49 ` Konrad Dybcio
2026-03-16 12:09 ` Bryan O'Donoghue
2026-03-16 7:42 ` Krzysztof Kozlowski
2026-03-16 21:31 ` Vijay Kumar Tumati
2026-03-17 5:26 ` Bryan O'Donoghue
2026-03-17 20:25 ` Vijay Kumar Tumati
2026-03-23 14:22 ` Konrad Dybcio
2026-03-23 14:29 ` Bryan O'Donoghue
2026-03-15 23:52 ` [PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver Bryan O'Donoghue
2026-03-16 10:12 ` Krzysztof Kozlowski
2026-03-16 12:04 ` Bryan O'Donoghue
2026-03-18 10:15 ` Neil Armstrong
2026-03-18 13:17 ` Bryan O'Donoghue
2026-03-18 15:07 ` Neil Armstrong
2026-03-18 15:27 ` Dmitry Baryshkov
2026-03-19 13:08 ` Vladimir Zapolskiy
2026-03-19 13:17 ` Bryan O'Donoghue
2026-03-19 14:05 ` Neil Armstrong
2026-03-19 15:06 ` Bryan O'Donoghue
2026-03-19 14:56 ` Vladimir Zapolskiy
2026-03-19 15:18 ` Bryan O'Donoghue
2026-03-19 16:08 ` Neil Armstrong
2026-03-19 16:56 ` Bryan O'Donoghue
2026-03-19 17:39 ` Neil Armstrong
2026-03-20 0:37 ` Vladimir Zapolskiy
2026-03-20 9:56 ` Bryan O'Donoghue
2026-03-18 15:47 ` Bryan O'Donoghue
2026-03-22 16:44 ` kernel test robot
2026-03-22 23:31 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox