Linux Media Controller development
 help / color / mirror / Atom feed
* [PATCH RFC 0/4] phy: qcom-mipi-csi2: C-PHY for X1E80100
@ 2026-10-15 22:20 George Neacsu via B4 Relay
  2026-10-15 22:20 ` [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options George Neacsu via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: George Neacsu via B4 Relay @ 2026-10-15 22:20 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel, George Neacsu

Hi Bryan, Vinod, David,

The Snapdragon Surface Pro 11's front camera, an IMX681, is on CSIPHY2
over one MIPI C-PHY trio. linux-next has the CSI2 PHY driver and the CAMSS
PHY API path, and C-PHY is the piece still missing for this camera. This
series adds it. Before I go further, I would like to know whether this is
the shape you want:

1. PHY_MODE_MIPI_CPHY and C-PHY configuration options in the generic PHY
   framework, because CAMSS had no way to ask the PHY for C-PHY;
2. C-PHY in qcom-mipi-csi2, with the X1E80100 sequence taken from
   Qualcomm's GPL-2.0 downstream header;
3. the CSID 680 PHY type select, which is defined but never written;
4. C-PHY in csiphy_stream_on() for PHY API CSIPHYs.

The base is next-20261015 plus David's "media: camss: Add support for
C-PHY configuration on Qualcomm platforms" v9, which provides phy_cfg,
the CSID phy_sel and the C-PHY link frequency. I had to port its 8/9 by
hand onto the PHY API changes:
https://lore.kernel.org/all/20260617-qcom-cphy-v9-0-83da8a8e4e44@ixit.cz/

Tested on the Surface Pro 11 (X1E80100). Both cameras run together through
the upstream stack: CSI2 PHY nodes as in Bryan's x1e80100 camera DT v7,
and CAMSS in PHY API mode, with raw capture over RDI:
- front IMX681 (Sergey's and Andre's v8 driver plus its C-PHY RFC) on
  CSIPHY2, one C-PHY trio at 2.4 Gsym/s: 3844x2640 at 30.01 fps, and the
  sensor's colour bars are byte-identical to the pattern captured with an
  out-of-tree driver;
- rear OV13858 on CSIPHY1, four D-PHY lanes: 29.9 fps, as before the
  series.
The kernel was 7.1.5, with linux-next's CAMSS and CSI2 PHY built as
modules; the test build needed shims only for API differences.

Two questions:
- Is a generic C-PHY mode how you want CAMSS to ask for it, or would you
  rather the PHY take the mode from its own endpoint alone?
- Qualcomm's X1E sequence is for 2.5 Gsps, and this sensor runs at
  2.4 Gsym/s with it. The sa8775p C-PHY series selects settings by data
  rate. Should X1E do the same?

Patches 1-2 are PHY and 3-4 are CAMSS. They can go separately once the
approach is agreed.

Thanks,
George

---
George Neacsu (4):
      phy: Add MIPI C-PHY mode and configuration options
      phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
      media: qcom: camss: csid-680: Select C-PHY in the receiver
      media: qcom: camss: Configure C-PHY through the PHY API

 drivers/media/platform/qcom/camss/camss-csid-680.c |   2 +
 drivers/media/platform/qcom/camss/camss-csiphy.c   |  35 +++--
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 151 ++++++++++++++++++++-
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     |  49 ++++++-
 drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |   4 +
 include/linux/phy/phy-mipi-cphy.h                  |  27 ++++
 include/linux/phy/phy.h                            |   5 +
 7 files changed, 252 insertions(+), 21 deletions(-)
---
base-commit: 4a926a0ea6c65089285eb3ccb84ac6b02b22dee5
change-id: 20261015-x1e-cphy-rfc-201860c8097f

Best regards,
-- 
George Neacsu <geocausa@gmail.com>



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options
  2026-10-15 22:20 [PATCH RFC 0/4] phy: qcom-mipi-csi2: C-PHY for X1E80100 George Neacsu via B4 Relay
@ 2026-10-15 22:20 ` George Neacsu via B4 Relay
  2026-10-16  9:13   ` Bryan O'Donoghue
  2026-10-16  9:24   ` Loic Poulain
  2026-10-15 22:20 ` [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100 George Neacsu via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 17+ messages in thread
From: George Neacsu via B4 Relay @ 2026-10-15 22:20 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel, George Neacsu

From: George Neacsu <geocausa@gmail.com>

The generic PHY framework has a mode and configuration options for MIPI
D-PHY but none for C-PHY, so a CSI-2 receiver has no way to ask a PHY
that does both for C-PHY. Add PHY_MODE_MIPI_CPHY and struct
phy_configure_opts_mipi_cphy. The options are the two values a receiver
knows: the symbol rate on each trio and the number of trios.

The first user is the Qualcomm CSI2 PHY, whose X1E80100 instances carry
the C-PHY front camera of the Surface Pro 11.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
 include/linux/phy/phy-mipi-cphy.h | 27 +++++++++++++++++++++++++++
 include/linux/phy/phy.h           |  5 +++++
 2 files changed, 32 insertions(+)

diff --git a/include/linux/phy/phy-mipi-cphy.h b/include/linux/phy/phy-mipi-cphy.h
new file mode 100644
index 000000000..e3f0a8257
--- /dev/null
+++ b/include/linux/phy/phy-mipi-cphy.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PHY_MIPI_CPHY_H_
+#define __PHY_MIPI_CPHY_H_
+
+/**
+ * struct phy_configure_opts_mipi_cphy - MIPI C-PHY configuration set
+ *
+ * This structure is used to represent the configuration state of a
+ * MIPI C-PHY phy.
+ */
+struct phy_configure_opts_mipi_cphy {
+	/**
+	 * @symbol_rate:
+	 *
+	 * Symbol rate on each trio, in symbols per second.
+	 */
+	unsigned long		symbol_rate;
+
+	/**
+	 * @lanes:
+	 *
+	 * Number of active trios.
+	 */
+	unsigned char		lanes;
+};
+
+#endif /* __PHY_MIPI_CPHY_H_ */
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index c480d6cb4..f12337834 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -19,6 +19,7 @@
 #include <linux/phy/phy-dp.h>
 #include <linux/phy/phy-hdmi.h>
 #include <linux/phy/phy-lvds.h>
+#include <linux/phy/phy-mipi-cphy.h>
 #include <linux/phy/phy-mipi-dphy.h>
 #include <linux/phy/phy-thunderbolt.h>
 
@@ -42,6 +43,7 @@ enum phy_mode {
 	PHY_MODE_PCIE,
 	PHY_MODE_ETHERNET,
 	PHY_MODE_MIPI_DPHY,
+	PHY_MODE_MIPI_CPHY,
 	PHY_MODE_SATA,
 	PHY_MODE_LVDS,
 	PHY_MODE_DP,
@@ -69,6 +71,8 @@ union phy_notify {
  *
  * @mipi_dphy:	Configuration set applicable for phys supporting
  *		the MIPI_DPHY phy mode.
+ * @mipi_cphy:	Configuration set applicable for phys supporting
+ *		the MIPI_CPHY phy mode.
  * @dp:		Configuration set applicable for phys supporting
  *		the DisplayPort protocol.
  * @lvds:	Configuration set applicable for phys supporting
@@ -78,6 +82,7 @@ union phy_notify {
  */
 union phy_configure_opts {
 	struct phy_configure_opts_mipi_dphy	mipi_dphy;
+	struct phy_configure_opts_mipi_cphy	mipi_cphy;
 	struct phy_configure_opts_dp		dp;
 	struct phy_configure_opts_lvds		lvds;
 	struct phy_configure_opts_hdmi		hdmi;

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
  2026-10-15 22:20 [PATCH RFC 0/4] phy: qcom-mipi-csi2: C-PHY for X1E80100 George Neacsu via B4 Relay
  2026-10-15 22:20 ` [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options George Neacsu via B4 Relay
@ 2026-10-15 22:20 ` George Neacsu via B4 Relay
  2026-10-16  9:29   ` Bryan O'Donoghue
                     ` (2 more replies)
  2026-10-15 22:20 ` [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the receiver George Neacsu via B4 Relay
  2026-10-15 22:20 ` [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API George Neacsu via B4 Relay
  3 siblings, 3 replies; 17+ messages in thread
From: George Neacsu via B4 Relay @ 2026-10-15 22:20 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel, George Neacsu

From: George Neacsu <geocausa@gmail.com>

The CSI2 PHY schema already allows bus-type C-PHY, but the driver
rejects it. Accept it on SoCs that provide a C-PHY sequence: up to three
trios and no lane polarities, and set_mode() has to ask for the mode the
endpoint describes. In C-PHY mode, configure() takes the symbol rate.
lanes_enable() powers up the trios on the odd CTRL5 bits and writes the
C-PHY sequence, which carries its own timing, so no settle count is
computed.

The X1E80100 sequence is Qualcomm's for the v2.1.2 3PH PHY at 2.5 Gsps,
taken from the downstream camera driver's GPL-2.0
cam_csiphy_2_1_2_hwreg.h (csiphy_3ph_v2_1_2_reg, datarate_212_2p5Gsps
and the 3PH reset exit), as the D-PHY sequence above it is.

Tested on the Surface Pro 11 (X1E80100) with an IMX681 on CSIPHY2, one
trio at 2.4 Gsym/s, and raw capture through CSID1 and an RDI interface:
3844x2640 at 30.01 fps, and the sensor's colour bars are byte-identical
to the same pattern captured with an out-of-tree driver. D-PHY is
unchanged: an OV13858 on CSIPHY1 over four lanes streams at 29.9 fps.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 151 ++++++++++++++++++++-
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     |  49 ++++++-
 drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |   4 +
 3 files changed, 197 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
index 966d79c98..e758b6755 100644
--- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2016-2026 Linaro Ltd.
  */
 
+#include <dt-bindings/media/video-interfaces.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -161,6 +162,131 @@ mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
 	{.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
 };
 
+/*
+ * 4nm 3PH v2.1.2 C-PHY, 2.5 Gsps, from Qualcomm's GPL-2.0 camera driver
+ * (cam_csiphy_2_1_2_hwreg.h: csiphy_3ph_v2_1_2_reg[], datarate_212_2p5Gsps[]
+ * and the 3PH reset-exit write), one trio block per lane triplet.
+ */
+static const struct
+mipi_csi2phy_lane_regs lane_regs_x1e80100_cphy[] = {
+	/* Power up lanes 3ph mode */
+	{.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	/* datarate_212_2p5Gsps: AFE settings */
+	{.reg_addr = 0x0268, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0294, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0288, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0278, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x026c, .reg_data = 0x3d, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x028c, .reg_data = 0x30, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0270, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0274, .reg_data = 0x03, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0668, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0694, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0688, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0678, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x066c, .reg_data = 0x3d, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x068c, .reg_data = 0x30, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0670, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0674, .reg_data = 0x03, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a68, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a94, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a88, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a78, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a6c, .reg_data = 0x3d, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a8c, .reg_data = 0x30, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a70, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a74, .reg_data = 0x03, .delay_us = 10, .param_type = CSIPHY_DEFAULT_PARAMS},
+	/* csiphy_3ph_v2_1_2_reg */
+	{.reg_addr = 0x02f4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x02f8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x02fc, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x02f0, .reg_data = 0xef, .delay_us = 3, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06f4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06f8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06fc, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06f0, .reg_data = 0xef, .delay_us = 3, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0af4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0af8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0afc, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0af0, .reg_data = 0xef, .delay_us = 3, .param_type = CSIPHY_DEFAULT_PARAMS},
+	/* datarate_212_2p5Gsps, then csiphy_3ph_v2_1_2_reg */
+	{.reg_addr = 0x020c, .reg_data = 0x22, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0208, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0210, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0214, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0204, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x02e4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x02e8, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x02ec, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0218, .reg_data = 0x3e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x021c, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0220, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0224, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0228, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x022c, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0264, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0244, .reg_data = 0xb2, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0310, .reg_data = 0x35, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x02bc, .reg_data = 0xd0, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0254, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0240, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0260, .reg_data = 0xa8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0284, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0290, .reg_data = 0x02, .param_type = CSIPHY_DEFAULT_PARAMS},
+	/* datarate_212_2p5Gsps, then csiphy_3ph_v2_1_2_reg */
+	{.reg_addr = 0x060c, .reg_data = 0x22, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0608, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0610, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0614, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0604, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06e4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06e8, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06ec, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0618, .reg_data = 0x3e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x061c, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0620, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0624, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0628, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x062c, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0664, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0644, .reg_data = 0xb2, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0710, .reg_data = 0x35, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x06bc, .reg_data = 0xd0, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0654, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0640, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0660, .reg_data = 0xa8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0684, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0690, .reg_data = 0x02, .param_type = CSIPHY_DEFAULT_PARAMS},
+	/* datarate_212_2p5Gsps, then csiphy_3ph_v2_1_2_reg */
+	{.reg_addr = 0x0a0c, .reg_data = 0x22, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a08, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a10, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a14, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a04, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0ae4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0ae8, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0aec, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a18, .reg_data = 0x3e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a1c, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a20, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a24, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a28, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a2c, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a64, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a44, .reg_data = 0xb2, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0b10, .reg_data = 0x35, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0abc, .reg_data = 0xd0, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a54, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a40, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a60, .reg_data = 0xa8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a84, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0a90, .reg_data = 0x02, .param_type = CSIPHY_DEFAULT_PARAMS},
+	/* csiphy_reset_exit_reg_2_1_2, 3PH */
+	{.reg_addr = 0x1000, .reg_data = 0x0e, .delay_us = 3048,
+	 .param_type = CSIPHY_DEFAULT_PARAMS},
+};
+
 static inline const struct mipi_csi2phy_device_regs *
 csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
 {
@@ -246,8 +372,11 @@ 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;
+	const bool cphy = csi2phy->bus_type == MEDIA_BUS_TYPE_CSI2_CPHY;
+	const struct mipi_csi2phy_lane_regs *r = cphy ? regs->cphy_init_seq :
+								regs->init_seq;
+	int i, array_size = cphy ? regs->cphy_lane_array_size :
+				   regs->lane_array_size;
 	u32 val;
 
 	for (i = 0; i < array_size; i++, r++) {
@@ -273,13 +402,26 @@ static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
 {
 	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;
+	const bool cphy = csi2phy->bus_type == MEDIA_BUS_TYPE_CSI2_CPHY;
+	u8 settle_cnt = 0;
 	u8 val;
 	int i;
 
 	if (cfg->link_freq <= 0)
 		return -EINVAL;
 
+	if (cphy) {
+		/* The C-PHY sequence carries its own timing; trios are the odd bits. */
+		val = 0;
+		for (i = 0; i < cfg->num_data_lanes; i++)
+			val |= BIT(lane_cfg->data[i].pos * 2 + 1);
+
+		writel(val, csi2phy->base +
+		       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
+
+		goto common;
+	}
+
 	settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
 	if (!settle_cnt)
 		return -ENODEV;
@@ -309,6 +451,7 @@ static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
 	if (lane_cfg->clk.pol)
 		writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(lane_cfg->clk.pos));
 
+common:
 	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));
@@ -374,6 +517,8 @@ const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
 	.reg_info = {
 		.init_seq = lane_regs_x1e80100,
 		.lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
+		.cphy_init_seq = lane_regs_x1e80100_cphy,
+		.cphy_lane_array_size = ARRAY_SIZE(lane_regs_x1e80100_cphy),
 		.common_regs_offset = 0x1000,
 	},
 	.supply_names = (const char **)x1e_supplies,
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
index c0a4fa20e..892c3da55 100644
--- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
@@ -104,6 +104,19 @@ static int phy_qcom_mipi_csi2_configure(struct phy *phy,
 	struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
 	int ret;
 
+	if (csi2phy->phy_mode == PHY_MODE_MIPI_CPHY) {
+		struct phy_configure_opts_mipi_cphy *cphy_cfg = &opts->mipi_cphy;
+
+		if (!cphy_cfg->symbol_rate ||
+		    cphy_cfg->lanes != stream_cfg->num_data_lanes)
+			return -EINVAL;
+
+		/* As for V4L2, a C-PHY link frequency is half the symbol rate. */
+		stream_cfg->link_freq = cphy_cfg->symbol_rate / 2;
+
+		return 0;
+	}
+
 	ret = phy_mipi_dphy_config_validate(dphy_cfg);
 	if (ret)
 		return ret;
@@ -124,8 +137,19 @@ static int phy_qcom_mipi_csi2_set_mode(struct phy *phy,
 {
 	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
 
-	if (mode != PHY_MODE_MIPI_DPHY)
+	/* The mode has to be the bus the endpoint describes. */
+	switch (mode) {
+	case PHY_MODE_MIPI_DPHY:
+		if (csi2phy->bus_type != MEDIA_BUS_TYPE_CSI2_DPHY)
+			return -EINVAL;
+		break;
+	case PHY_MODE_MIPI_CPHY:
+		if (csi2phy->bus_type != MEDIA_BUS_TYPE_CSI2_CPHY)
+			return -EINVAL;
+		break;
+	default:
 		return -EOPNOTSUPP;
+	}
 
 	csi2phy->phy_mode = mode;
 	return 0;
@@ -258,6 +282,7 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
 	struct fwnode_handle *ep;
 	int num_polarities;
 	int num_data_lanes;
+	int max_lanes;
 	u32 bus_type;
 	int i, ret;
 
@@ -278,14 +303,25 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
 	bus_type = MEDIA_BUS_TYPE_CSI2_DPHY;
 	fwnode_property_read_u32(ep, "bus-type", &bus_type);
 
-	if (bus_type != MEDIA_BUS_TYPE_CSI2_DPHY) {
+	switch (bus_type) {
+	case MEDIA_BUS_TYPE_CSI2_DPHY:
+		max_lanes = CSI2_MAX_DATA_LANES;
+		break;
+	case MEDIA_BUS_TYPE_CSI2_CPHY:
+		if (csi2phy->soc_cfg->reg_info.cphy_init_seq) {
+			max_lanes = CSI2_MAX_TRIOS;
+			break;
+		}
+		fallthrough;
+	default:
 		ret = -EOPNOTSUPP;
 		dev_err(dev, "Unsupported bus-type %u\n", bus_type);
 		goto out_put;
 	}
+	csi2phy->bus_type = bus_type;
 
 	num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
-	if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
+	if (num_data_lanes < 1 || num_data_lanes > max_lanes) {
 		ret = -EINVAL;
 		dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
 		goto out_put;
@@ -302,6 +338,11 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
 	/* lane-polarities: optional, up to num_data_lanes + 1 entries */
 	memset(lane_polarities, 0x00, sizeof(lane_polarities));
 	num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
+	if (num_polarities > 0 && bus_type == MEDIA_BUS_TYPE_CSI2_CPHY) {
+		ret = -EINVAL;
+		dev_err(dev, "lane-polarities do not apply to C-PHY\n");
+		goto out_put;
+	}
 	if (num_polarities > 0) {
 		if (num_polarities != stream_cfg->num_data_lanes + 1) {
 			ret = -EINVAL;
@@ -322,7 +363,7 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
 	csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
 
 	for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
-		if (data_lanes[i] < 1 || data_lanes[i] > CSI2_MAX_DATA_LANES) {
+		if (data_lanes[i] < 1 || data_lanes[i] > max_lanes) {
 			dev_err(dev, "Invalid lane %d\n", data_lanes[i]);
 			ret = -EINVAL;
 			goto out_put;
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
index 7e55ae007..442c4d6fe 100644
--- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
@@ -11,6 +11,7 @@
 #include <linux/phy/phy.h>
 
 #define CSI2_MAX_DATA_LANES	4
+#define CSI2_MAX_TRIOS		3
 #define CSI2_DEFAULT_CLK_LANE	7
 
 struct mipi_csi2phy_lane {
@@ -50,6 +51,8 @@ struct mipi_csi2phy_lane_regs {
 struct mipi_csi2phy_device_regs {
 	const struct mipi_csi2phy_lane_regs *init_seq;
 	const int lane_array_size;
+	const struct mipi_csi2phy_lane_regs *cphy_init_seq;
+	const int cphy_lane_array_size;
 	const u32 common_regs_offset;
 };
 
@@ -75,6 +78,7 @@ struct mipi_csi2phy_soc_cfg {
 struct mipi_csi2phy_device {
 	struct device *dev;
 	u8 phy_mode;
+	u32 bus_type;
 
 	struct phy *phy;
 	void __iomem *base;

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the receiver
  2026-10-15 22:20 [PATCH RFC 0/4] phy: qcom-mipi-csi2: C-PHY for X1E80100 George Neacsu via B4 Relay
  2026-10-15 22:20 ` [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options George Neacsu via B4 Relay
  2026-10-15 22:20 ` [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100 George Neacsu via B4 Relay
@ 2026-10-15 22:20 ` George Neacsu via B4 Relay
  2026-10-16  7:50   ` Loic Poulain
  2026-10-15 22:20 ` [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API George Neacsu via B4 Relay
  3 siblings, 1 reply; 17+ messages in thread
From: George Neacsu via B4 Relay @ 2026-10-15 22:20 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel, George Neacsu

From: George Neacsu <geocausa@gmail.com>

CSID 680 has a PHY type select in CSI2_RX_CFG0, next to the PHY number.
The field is defined but never written, so the receiver always decodes
D-PHY. Write phy_sel to it, as csid-gen2 does for C-PHY.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
 drivers/media/platform/qcom/camss/camss-csid-680.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
index 345a67c8f..16b199f06 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-680.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
@@ -202,6 +202,8 @@ static void __csid_configure_rx(struct csid_device *csid,
 			<< CSI2_RX_CFG0_PHY_NUM_SEL;
 	}
 
+	val |= phy->phy_sel << CSI2_RX_CFG0_PHY_TYPE_SEL;
+
 	writel(val, csid->base + CSID_CSI2_RX_CFG0);
 
 	val = CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API
  2026-10-15 22:20 [PATCH RFC 0/4] phy: qcom-mipi-csi2: C-PHY for X1E80100 George Neacsu via B4 Relay
                   ` (2 preceding siblings ...)
  2026-10-15 22:20 ` [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the receiver George Neacsu via B4 Relay
@ 2026-10-15 22:20 ` George Neacsu via B4 Relay
  2026-10-16  7:56   ` Loic Poulain
                     ` (2 more replies)
  3 siblings, 3 replies; 17+ messages in thread
From: George Neacsu via B4 Relay @ 2026-10-15 22:20 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel, George Neacsu

From: George Neacsu <geocausa@gmail.com>

With a PHY API CSIPHY, csiphy_stream_on() always asks for D-PHY. Ask for
C-PHY when the endpoint is C-PHY, and pass the trio count and the symbol
rate, which by the V4L2 convention is twice the link frequency.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
 drivers/media/platform/qcom/camss/camss-csiphy.c | 35 ++++++++++++++----------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index f44f2aeb9..8e1b4335a 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -329,17 +329,15 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
 	u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
 				csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
 	struct csiphy_lanes_cfg *lncfg = &csiphy->cfg.csi2->lane_cfg;
-	struct phy_configure_opts_mipi_dphy *dphy_cfg;
-	union phy_configure_opts dphy_opts = { 0 };
+	const bool cphy = lncfg->phy_cfg == V4L2_MBUS_CSI2_CPHY;
+	union phy_configure_opts opts = { 0 };
 	struct device *dev = csiphy->camss->dev;
 	u8 num_lanes = lncfg->num_data;
 	s64 link_freq;
 	int ret;
 
-	dphy_cfg = &dphy_opts.mipi_dphy;
-
 	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes,
-					csiphy->cfg.csi2->lane_cfg.phy_cfg == V4L2_MBUS_CSI2_CPHY);
+					cphy);
 
 	if (link_freq <= 0) {
 		dev_err(dev,
@@ -347,23 +345,32 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
 		return -EINVAL;
 	}
 
-	ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
-							 dphy_cfg);
-	if (ret < 0) {
-		dev_err(dev, "Invalid config for link frequency %lld\n",
-			link_freq);
-		return ret;
+	if (cphy) {
+		/* A C-PHY link frequency is half the symbol rate. */
+		opts.mipi_cphy.symbol_rate = link_freq * 2;
+		opts.mipi_cphy.lanes = num_lanes;
+	} else {
+		ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq,
+								 num_lanes,
+								 &opts.mipi_dphy);
+		if (ret < 0) {
+			dev_err(dev, "Invalid config for link frequency %lld\n",
+				link_freq);
+			return ret;
+		}
 	}
 
-	ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
+	ret = phy_set_mode(csiphy->phy,
+			   cphy ? PHY_MODE_MIPI_CPHY : PHY_MODE_MIPI_DPHY);
 	if (ret) {
 		dev_err(dev, "Failed to set phy mode\n");
 		return ret;
 	}
 
-	ret = phy_configure(csiphy->phy, &dphy_opts);
+	ret = phy_configure(csiphy->phy, &opts);
 	if (ret) {
-		dev_err(dev, "failed to configure MIPI D-PHY\n");
+		dev_err(dev, "failed to configure MIPI %s\n",
+			cphy ? "C-PHY" : "D-PHY");
 		goto error;
 	}
 

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the receiver
  2026-10-15 22:20 ` [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the receiver George Neacsu via B4 Relay
@ 2026-10-16  7:50   ` Loic Poulain
  0 siblings, 0 replies; 17+ messages in thread
From: Loic Poulain @ 2026-10-16  7:50 UTC (permalink / raw)
  To: geocausa
  Cc: Bryan O'Donoghue, Vinod Koul, David Heidelberg,
	Neil Armstrong, Vladimir Zapolskiy, Mauro Carvalho Chehab,
	Manivannan Sadhasivam, linux-phy, linux-media, linux-arm-msm,
	linux-kernel

On Fri, Oct 16, 2026 at 12:20 AM George Neacsu via B4 Relay
<devnull+geocausa.gmail.com@kernel.org> wrote:
>
> From: George Neacsu <geocausa@gmail.com>
>
> CSID 680 has a PHY type select in CSI2_RX_CFG0, next to the PHY number.
> The field is defined but never written, so the receiver always decodes
> D-PHY. Write phy_sel to it, as csid-gen2 does for C-PHY.
>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>

It would be even better with proper masking/FIELD_PREP.

Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

> ---
>  drivers/media/platform/qcom/camss/camss-csid-680.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/drivers/media/platform/qcom/camss/camss-csid-680.c
> index 345a67c8f..16b199f06 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-680.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
> @@ -202,6 +202,8 @@ static void __csid_configure_rx(struct csid_device *csid,
>                         << CSI2_RX_CFG0_PHY_NUM_SEL;
>         }
>
> +       val |= phy->phy_sel << CSI2_RX_CFG0_PHY_TYPE_SEL;
> +
>         writel(val, csid->base + CSID_CSI2_RX_CFG0);
>
>         val = CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
>
> --
> 2.53.0
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API
  2026-10-15 22:20 ` [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API George Neacsu via B4 Relay
@ 2026-10-16  7:56   ` Loic Poulain
  2026-10-16  8:59   ` Bryan O'Donoghue
  2026-10-16  9:09   ` Bryan O'Donoghue
  2 siblings, 0 replies; 17+ messages in thread
From: Loic Poulain @ 2026-10-16  7:56 UTC (permalink / raw)
  To: geocausa
  Cc: Bryan O'Donoghue, Vinod Koul, David Heidelberg,
	Neil Armstrong, Vladimir Zapolskiy, Mauro Carvalho Chehab,
	Manivannan Sadhasivam, linux-phy, linux-media, linux-arm-msm,
	linux-kernel

On Fri, Oct 16, 2026 at 12:20 AM George Neacsu via B4 Relay
<devnull+geocausa.gmail.com@kernel.org> wrote:
>
> From: George Neacsu <geocausa@gmail.com>
>
> With a PHY API CSIPHY, csiphy_stream_on() always asks for D-PHY. Ask for
> C-PHY when the endpoint is C-PHY, and pass the trio count and the symbol
> rate, which by the V4L2 convention is twice the link frequency.
>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
> ---
>  drivers/media/platform/qcom/camss/camss-csiphy.c | 35 ++++++++++++++----------
>  1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index f44f2aeb9..8e1b4335a 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -329,17 +329,15 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>         u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
>                                 csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
>         struct csiphy_lanes_cfg *lncfg = &csiphy->cfg.csi2->lane_cfg;
> -       struct phy_configure_opts_mipi_dphy *dphy_cfg;
> -       union phy_configure_opts dphy_opts = { 0 };
> +       const bool cphy = lncfg->phy_cfg == V4L2_MBUS_CSI2_CPHY;
> +       union phy_configure_opts opts = { 0 };
>         struct device *dev = csiphy->camss->dev;
>         u8 num_lanes = lncfg->num_data;
>         s64 link_freq;
>         int ret;
>
> -       dphy_cfg = &dphy_opts.mipi_dphy;
> -
>         link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes,
> -                                       csiphy->cfg.csi2->lane_cfg.phy_cfg == V4L2_MBUS_CSI2_CPHY);
> +                                       cphy);
>
>         if (link_freq <= 0) {
>                 dev_err(dev,
> @@ -347,23 +345,32 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>                 return -EINVAL;
>         }
>
> -       ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
> -                                                        dphy_cfg);
> -       if (ret < 0) {
> -               dev_err(dev, "Invalid config for link frequency %lld\n",
> -                       link_freq);
> -               return ret;
> +       if (cphy) {
> +               /* A C-PHY link frequency is half the symbol rate. */
> +               opts.mipi_cphy.symbol_rate = link_freq * 2;
> +               opts.mipi_cphy.lanes = num_lanes;

Wouldn't it be cleaner to use a similar helper function as the D-PHY
driver does:
Introducing a phy_mipi_cphy_get_default_config_for_hsclk

> +       } else {
> +               ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq,
> +                                                                num_lanes,
> +                                                                &opts.mipi_dphy);
> +               if (ret < 0) {
> +                       dev_err(dev, "Invalid config for link frequency %lld\n",
> +                               link_freq);
> +                       return ret;
> +               }
>         }
>
> -       ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
> +       ret = phy_set_mode(csiphy->phy,
> +                          cphy ? PHY_MODE_MIPI_CPHY : PHY_MODE_MIPI_DPHY);
>         if (ret) {
>                 dev_err(dev, "Failed to set phy mode\n");
>                 return ret;
>         }
>
> -       ret = phy_configure(csiphy->phy, &dphy_opts);
> +       ret = phy_configure(csiphy->phy, &opts);
>         if (ret) {
> -               dev_err(dev, "failed to configure MIPI D-PHY\n");
> +               dev_err(dev, "failed to configure MIPI %s\n",
> +                       cphy ? "C-PHY" : "D-PHY");
>                 goto error;
>         }
>
>
> --
> 2.53.0
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API
  2026-10-15 22:20 ` [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API George Neacsu via B4 Relay
  2026-10-16  7:56   ` Loic Poulain
@ 2026-10-16  8:59   ` Bryan O'Donoghue
  2026-10-16  9:09   ` Bryan O'Donoghue
  2 siblings, 0 replies; 17+ messages in thread
From: Bryan O'Donoghue @ 2026-10-16  8:59 UTC (permalink / raw)
  To: geocausa, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel

On 15/10/2026 23:20, George Neacsu via B4 Relay wrote:
> From: George Neacsu <geocausa@gmail.com>
> 
> With a PHY API CSIPHY, csiphy_stream_on() always asks for D-PHY. Ask for
> C-PHY when the endpoint is C-PHY, and pass the trio count and the symbol
> rate, which by the V4L2 convention is twice the link frequency.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
Please rebase off of -next this patch doesn't apply

[~/Development/worktree/bod-media-committers]$ git am --3way < 
./20261015_geocausa_phy_qcom_mipi_csi2_c_phy_for_x1e80100.mbx 
                                       *[bod-qcom-media-committers]
Applying: phy: Add MIPI C-PHY mode and configuration options
Applying: phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
Applying: media: qcom: camss: csid-680: Select C-PHY in the receiver
Applying: media: qcom: camss: Configure C-PHY through the PHY API
error: sha1 information is lacking or useless 
(drivers/media/platform/qcom/camss/camss-csiphy.c).
error: could not build fake ancestor
Patch failed at 0004 media: qcom: camss: Configure C-PHY through the PHY API
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am 
--abort".
hint: Disable this message with "git config set advice.mergeConflict false"

---
bod

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API
  2026-10-15 22:20 ` [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API George Neacsu via B4 Relay
  2026-10-16  7:56   ` Loic Poulain
  2026-10-16  8:59   ` Bryan O'Donoghue
@ 2026-10-16  9:09   ` Bryan O'Donoghue
  2 siblings, 0 replies; 17+ messages in thread
From: Bryan O'Donoghue @ 2026-10-16  9:09 UTC (permalink / raw)
  To: geocausa, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel

On 15/10/2026 23:20, George Neacsu via B4 Relay wrote:
> From: George Neacsu <geocausa@gmail.com>
> 
> With a PHY API CSIPHY, csiphy_stream_on() always asks for D-PHY. Ask for
> C-PHY when the endpoint is C-PHY, and pass the trio count and the symbol
> rate, which by the V4L2 convention is twice the link frequency.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
> ---
>   drivers/media/platform/qcom/camss/camss-csiphy.c | 35 ++++++++++++++----------
>   1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index f44f2aeb9..8e1b4335a 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -329,17 +329,15 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>   	u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
>   				csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
>   	struct csiphy_lanes_cfg *lncfg = &csiphy->cfg.csi2->lane_cfg;
> -	struct phy_configure_opts_mipi_dphy *dphy_cfg;
> -	union phy_configure_opts dphy_opts = { 0 };
> +	const bool cphy = lncfg->phy_cfg == V4L2_MBUS_CSI2_CPHY;
> +	union phy_configure_opts opts = { 0 };
>   	struct device *dev = csiphy->camss->dev;
>   	u8 num_lanes = lncfg->num_data;
>   	s64 link_freq;
>   	int ret;
> 
> -	dphy_cfg = &dphy_opts.mipi_dphy;
> -
>   	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes,
> -					csiphy->cfg.csi2->lane_cfg.phy_cfg == V4L2_MBUS_CSI2_CPHY);
> +					cphy);

That's a nice reduction, yikes who let that statement into the kernel :)

> 
>   	if (link_freq <= 0) {
>   		dev_err(dev,
> @@ -347,23 +345,32 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>   		return -EINVAL;
>   	}
> 
> -	ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
> -							 dphy_cfg);
> -	if (ret < 0) {
> -		dev_err(dev, "Invalid config for link frequency %lld\n",
> -			link_freq);
> -		return ret;
> +	if (cphy) {
> +		/* A C-PHY link frequency is half the symbol rate. */

A comment that should live in a helper.

> +		opts.mipi_cphy.symbol_rate = link_freq * 2;

Sounds like something which should have a helper coming from the PHY layer.

The analogue for DPHY is phy_mipi_dphy_get_default_config_for_hsclk()

> +		opts.mipi_cphy.lanes = num_lanes;
> +	} else {
> +		ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq,
> +								 num_lanes,
> +								 &opts.mipi_dphy);
> +		if (ret < 0) {
> +			dev_err(dev, "Invalid config for link frequency %lld\n",
> +				link_freq);
> +			return ret;
> +		}
>   	}
> 
> -	ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
> +	ret = phy_set_mode(csiphy->phy,
> +			   cphy ? PHY_MODE_MIPI_CPHY : PHY_MODE_MIPI_DPHY);

Instead of a bool if you define an int which carriers the type you can 
still disjunct on that all you like but ultimately pass it into 
phy_set_mode() directly - skipping at least this ternary statement.

>   	if (ret) {
>   		dev_err(dev, "Failed to set phy mode\n");
>   		return ret;
>   	}
> 
> -	ret = phy_configure(csiphy->phy, &dphy_opts);
> +	ret = phy_configure(csiphy->phy, &opts);
>   	if (ret) {
> -		dev_err(dev, "failed to configure MIPI D-PHY\n");
> +		dev_err(dev, "failed to configure MIPI %s\n",
> +			cphy ? "C-PHY" : "D-PHY");
>   		goto error;
>   	}
> 
> 
> --
> 2.53.0

---
bod

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options
  2026-10-15 22:20 ` [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options George Neacsu via B4 Relay
@ 2026-10-16  9:13   ` Bryan O'Donoghue
  2026-10-16  9:24   ` Loic Poulain
  1 sibling, 0 replies; 17+ messages in thread
From: Bryan O'Donoghue @ 2026-10-16  9:13 UTC (permalink / raw)
  To: geocausa, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel

On 15/10/2026 23:20, George Neacsu via B4 Relay wrote:
> From: George Neacsu <geocausa@gmail.com>
> 
> The generic PHY framework has a mode and configuration options for MIPI
> D-PHY but none for C-PHY, so a CSI-2 receiver has no way to ask a PHY
> that does both for C-PHY. Add PHY_MODE_MIPI_CPHY and struct
> phy_configure_opts_mipi_cphy. The options are the two values a receiver
> knows: the symbol rate on each trio and the number of trios.
> 
> The first user is the Qualcomm CSI2 PHY, whose X1E80100 instances carry
> the C-PHY front camera of the Surface Pro 11.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
> ---
>   include/linux/phy/phy-mipi-cphy.h | 27 +++++++++++++++++++++++++++
>   include/linux/phy/phy.h           |  5 +++++
>   2 files changed, 32 insertions(+)
> 
> diff --git a/include/linux/phy/phy-mipi-cphy.h b/include/linux/phy/phy-mipi-cphy.h
> new file mode 100644
> index 000000000..e3f0a8257
> --- /dev/null
> +++ b/include/linux/phy/phy-mipi-cphy.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PHY_MIPI_CPHY_H_
> +#define __PHY_MIPI_CPHY_H_
> +
> +/**
> + * struct phy_configure_opts_mipi_cphy - MIPI C-PHY configuration set
> + *
> + * This structure is used to represent the configuration state of a
> + * MIPI C-PHY phy.
> + */
> +struct phy_configure_opts_mipi_cphy {
> +	/**
> +	 * @symbol_rate:
> +	 *
> +	 * Symbol rate on each trio, in symbols per second.
> +	 */
> +	unsigned long		symbol_rate;
> +
> +	/**
> +	 * @lanes:
> +	 *
> +	 * Number of active trios.
> +	 */
> +	unsigned char		lanes;
> +};
> +
> +#endif /* __PHY_MIPI_CPHY_H_ */
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index c480d6cb4..f12337834 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -19,6 +19,7 @@
>   #include <linux/phy/phy-dp.h>
>   #include <linux/phy/phy-hdmi.h>
>   #include <linux/phy/phy-lvds.h>
> +#include <linux/phy/phy-mipi-cphy.h>
>   #include <linux/phy/phy-mipi-dphy.h>
>   #include <linux/phy/phy-thunderbolt.h>
> 
> @@ -42,6 +43,7 @@ enum phy_mode {
>   	PHY_MODE_PCIE,
>   	PHY_MODE_ETHERNET,
>   	PHY_MODE_MIPI_DPHY,
> +	PHY_MODE_MIPI_CPHY,
>   	PHY_MODE_SATA,
>   	PHY_MODE_LVDS,
>   	PHY_MODE_DP,
> @@ -69,6 +71,8 @@ union phy_notify {
>    *
>    * @mipi_dphy:	Configuration set applicable for phys supporting
>    *		the MIPI_DPHY phy mode.
> + * @mipi_cphy:	Configuration set applicable for phys supporting
> + *		the MIPI_CPHY phy mode.
>    * @dp:		Configuration set applicable for phys supporting
>    *		the DisplayPort protocol.
>    * @lvds:	Configuration set applicable for phys supporting
> @@ -78,6 +82,7 @@ union phy_notify {
>    */
>   union phy_configure_opts {
>   	struct phy_configure_opts_mipi_dphy	mipi_dphy;
> +	struct phy_configure_opts_mipi_cphy	mipi_cphy;
>   	struct phy_configure_opts_dp		dp;
>   	struct phy_configure_opts_lvds		lvds;
>   	struct phy_configure_opts_hdmi		hdmi;
> 
> --
> 2.53.0

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options
  2026-10-15 22:20 ` [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options George Neacsu via B4 Relay
  2026-10-16  9:13   ` Bryan O'Donoghue
@ 2026-10-16  9:24   ` Loic Poulain
  1 sibling, 0 replies; 17+ messages in thread
From: Loic Poulain @ 2026-10-16  9:24 UTC (permalink / raw)
  To: geocausa
  Cc: Bryan O'Donoghue, Vinod Koul, David Heidelberg,
	Neil Armstrong, Vladimir Zapolskiy, Mauro Carvalho Chehab,
	Manivannan Sadhasivam, linux-phy, linux-media, linux-arm-msm,
	linux-kernel

On Fri, Oct 16, 2026 at 12:20 AM George Neacsu via B4 Relay
<devnull+geocausa.gmail.com@kernel.org> wrote:
>
> From: George Neacsu <geocausa@gmail.com>
>
> The generic PHY framework has a mode and configuration options for MIPI
> D-PHY but none for C-PHY, so a CSI-2 receiver has no way to ask a PHY
> that does both for C-PHY. Add PHY_MODE_MIPI_CPHY and struct
> phy_configure_opts_mipi_cphy. The options are the two values a receiver
> knows: the symbol rate on each trio and the number of trios.
>
> The first user is the Qualcomm CSI2 PHY, whose X1E80100 instances carry
> the C-PHY front camera of the Surface Pro 11.
>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
> ---
>  include/linux/phy/phy-mipi-cphy.h | 27 +++++++++++++++++++++++++++
>  include/linux/phy/phy.h           |  5 +++++
>  2 files changed, 32 insertions(+)
>
> diff --git a/include/linux/phy/phy-mipi-cphy.h b/include/linux/phy/phy-mipi-cphy.h
> new file mode 100644
> index 000000000..e3f0a8257
> --- /dev/null
> +++ b/include/linux/phy/phy-mipi-cphy.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PHY_MIPI_CPHY_H_
> +#define __PHY_MIPI_CPHY_H_
> +
> +/**
> + * struct phy_configure_opts_mipi_cphy - MIPI C-PHY configuration set
> + *
> + * This structure is used to represent the configuration state of a
> + * MIPI C-PHY phy.
> + */
> +struct phy_configure_opts_mipi_cphy {
> +       /**
> +        * @symbol_rate:
> +        *
> +        * Symbol rate on each trio, in symbols per second.
> +        */
> +       unsigned long           symbol_rate;
> +
> +       /**
> +        * @lanes:
> +        *
> +        * Number of active trios.
> +        */
> +       unsigned char           lanes;

So, wouldn't 'trios' be a more accurate and specific name for this variable?

> +
> +#endif /* __PHY_MIPI_CPHY_H_ */
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index c480d6cb4..f12337834 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -19,6 +19,7 @@
>  #include <linux/phy/phy-dp.h>
>  #include <linux/phy/phy-hdmi.h>
>  #include <linux/phy/phy-lvds.h>
> +#include <linux/phy/phy-mipi-cphy.h>
>  #include <linux/phy/phy-mipi-dphy.h>
>  #include <linux/phy/phy-thunderbolt.h>
>
> @@ -42,6 +43,7 @@ enum phy_mode {
>         PHY_MODE_PCIE,
>         PHY_MODE_ETHERNET,
>         PHY_MODE_MIPI_DPHY,
> +       PHY_MODE_MIPI_CPHY,
>         PHY_MODE_SATA,
>         PHY_MODE_LVDS,
>         PHY_MODE_DP,
> @@ -69,6 +71,8 @@ union phy_notify {
>   *
>   * @mipi_dphy: Configuration set applicable for phys supporting
>   *             the MIPI_DPHY phy mode.
> + * @mipi_cphy: Configuration set applicable for phys supporting
> + *             the MIPI_CPHY phy mode.
>   * @dp:                Configuration set applicable for phys supporting
>   *             the DisplayPort protocol.
>   * @lvds:      Configuration set applicable for phys supporting
> @@ -78,6 +82,7 @@ union phy_notify {
>   */
>  union phy_configure_opts {
>         struct phy_configure_opts_mipi_dphy     mipi_dphy;
> +       struct phy_configure_opts_mipi_cphy     mipi_cphy;
>         struct phy_configure_opts_dp            dp;
>         struct phy_configure_opts_lvds          lvds;
>         struct phy_configure_opts_hdmi          hdmi;
>
> --
> 2.53.0
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
  2026-10-15 22:20 ` [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100 George Neacsu via B4 Relay
@ 2026-10-16  9:29   ` Bryan O'Donoghue
  2026-10-16 10:06     ` George Neacsu
  2026-10-16  9:59   ` Nihal Kumar Gupta
  2026-10-16 10:23   ` Konrad Dybcio
  2 siblings, 1 reply; 17+ messages in thread
From: Bryan O'Donoghue @ 2026-10-16  9:29 UTC (permalink / raw)
  To: geocausa, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel

On 15/10/2026 23:20, George Neacsu via B4 Relay wrote:
> From: George Neacsu <geocausa@gmail.com>
> 
> The CSI2 PHY schema already allows bus-type C-PHY, but the driver
> rejects it. Accept it on SoCs that provide a C-PHY sequence: up to three
> trios and no lane polarities, and set_mode() has to ask for the mode the
> endpoint describes. In C-PHY mode, configure() takes the symbol rate.
> lanes_enable() powers up the trios on the odd CTRL5 bits and writes the
> C-PHY sequence, which carries its own timing, so no settle count is
> computed.
> 
> The X1E80100 sequence is Qualcomm's for the v2.1.2 3PH PHY at 2.5 Gsps,
> taken from the downstream camera driver's GPL-2.0
> cam_csiphy_2_1_2_hwreg.h (csiphy_3ph_v2_1_2_reg, datarate_212_2p5Gsps
> and the 3PH reset exit), as the D-PHY sequence above it is.
> 
> Tested on the Surface Pro 11 (X1E80100) with an IMX681 on CSIPHY2, one
> trio at 2.4 Gsym/s, and raw capture through CSID1 and an RDI interface:
> 3844x2640 at 30.01 fps, and the sensor's colour bars are byte-identical
> to the same pattern captured with an out-of-tree driver. D-PHY is
> unchanged: an OV13858 on CSIPHY1 over four lanes streams at 29.9 fps.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
> ---
>   drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 151 ++++++++++++++++++++-
>   drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     |  49 ++++++-
>   drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |   4 +
>   3 files changed, 197 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> index 966d79c98..e758b6755 100644
> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> @@ -6,6 +6,7 @@
>    * Copyright (C) 2016-2026 Linaro Ltd.
>    */
> 
> +#include <dt-bindings/media/video-interfaces.h>

This is the first place your patch doesn't apply.

>   #include <linux/delay.h>
>   #include <linux/interrupt.h>
>   #include <linux/io.h>
> @@ -161,6 +162,131 @@ mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
>   	{.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
>   };
> 
> +/*
> + * 4nm 3PH v2.1.2 C-PHY, 2.5 Gsps, from Qualcomm's GPL-2.0 camera driver
> + * (cam_csiphy_2_1_2_hwreg.h: csiphy_3ph_v2_1_2_reg[], datarate_212_2p5Gsps[]
> + * and the 3PH reset-exit write), one trio block per lane triplet.
> + */
> +static const struct
> +mipi_csi2phy_lane_regs lane_regs_x1e80100_cphy[] = {
> +	/* Power up lanes 3ph mode */
> +	{.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* datarate_212_2p5Gsps: AFE settings */
> +	{.reg_addr = 0x0268, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0294, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0288, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0278, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x026c, .reg_data = 0x3d, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x028c, .reg_data = 0x30, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0270, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0274, .reg_data = 0x03, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0668, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0694, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0688, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0678, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x066c, .reg_data = 0x3d, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x068c, .reg_data = 0x30, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0670, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0674, .reg_data = 0x03, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a68, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a94, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a88, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a78, .reg_data = 0x20, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a6c, .reg_data = 0x3d, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a8c, .reg_data = 0x30, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a70, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a74, .reg_data = 0x03, .delay_us = 10, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* csiphy_3ph_v2_1_2_reg */
> +	{.reg_addr = 0x02f4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x02f8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x02fc, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x02f0, .reg_data = 0xef, .delay_us = 3, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06f4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06f8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06fc, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06f0, .reg_data = 0xef, .delay_us = 3, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0af4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0af8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0afc, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0af0, .reg_data = 0xef, .delay_us = 3, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* datarate_212_2p5Gsps, then csiphy_3ph_v2_1_2_reg */
> +	{.reg_addr = 0x020c, .reg_data = 0x22, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0208, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0210, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0214, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0204, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x02e4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x02e8, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x02ec, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0218, .reg_data = 0x3e, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x021c, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0220, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0224, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0228, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x022c, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0264, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0244, .reg_data = 0xb2, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0310, .reg_data = 0x35, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x02bc, .reg_data = 0xd0, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0254, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0240, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0260, .reg_data = 0xa8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0284, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0290, .reg_data = 0x02, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* datarate_212_2p5Gsps, then csiphy_3ph_v2_1_2_reg */
> +	{.reg_addr = 0x060c, .reg_data = 0x22, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0608, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0610, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0614, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0604, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06e4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06e8, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06ec, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0618, .reg_data = 0x3e, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x061c, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0620, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0624, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0628, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x062c, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0664, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0644, .reg_data = 0xb2, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0710, .reg_data = 0x35, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06bc, .reg_data = 0xd0, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0654, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0640, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0660, .reg_data = 0xa8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0684, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0690, .reg_data = 0x02, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* datarate_212_2p5Gsps, then csiphy_3ph_v2_1_2_reg */
> +	{.reg_addr = 0x0a0c, .reg_data = 0x22, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a08, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a10, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a14, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a04, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0ae4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0ae8, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0aec, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a18, .reg_data = 0x3e, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a1c, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a20, .reg_data = 0x41, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a24, .reg_data = 0x7f, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a28, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a2c, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a64, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a44, .reg_data = 0xb2, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0b10, .reg_data = 0x35, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0abc, .reg_data = 0xd0, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a54, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a40, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a60, .reg_data = 0xa8, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a84, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a90, .reg_data = 0x02, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* csiphy_reset_exit_reg_2_1_2, 3PH */
> +	{.reg_addr = 0x1000, .reg_data = 0x0e, .delay_us = 3048,
> +	 .param_type = CSIPHY_DEFAULT_PARAMS},
> +};
> +
>   static inline const struct mipi_csi2phy_device_regs *
>   csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
>   {
> @@ -246,8 +372,11 @@ 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;
> +	const bool cphy = csi2phy->bus_type == MEDIA_BUS_TYPE_CSI2_CPHY;
> +	const struct mipi_csi2phy_lane_regs *r = cphy ? regs->cphy_init_seq :
> +								regs->init_seq;

This hurts my eyes to read - just derive the pointer below with an if/else

> +	int i, array_size = cphy ? regs->cphy_lane_array_size :
> +				   regs->lane_array_size;

No declare the array_size on its own line and then calculate it.

>   	u32 val;
> 
>   	for (i = 0; i < array_size; i++, r++) {
> @@ -273,13 +402,26 @@ static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
>   {
>   	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;
> +	const bool cphy = csi2phy->bus_type == MEDIA_BUS_TYPE_CSI2_CPHY;
> +	u8 settle_cnt = 0;
>   	u8 val;
>   	int i;
> 
>   	if (cfg->link_freq <= 0)
>   		return -EINVAL;
> 
> +	if (cphy) {
> +		/* The C-PHY sequence carries its own timing; trios are the odd bits. */
> +		val = 0;
> +		for (i = 0; i < cfg->num_data_lanes; i++)
> +			val |= BIT(lane_cfg->data[i].pos * 2 + 1);
> +
> +		writel(val, csi2phy->base +
> +		       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> +
> +		goto common;

no gotos for this.

Functional decomposition please.

> +	}
> +
>   	settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
>   	if (!settle_cnt)
>   		return -ENODEV;
> @@ -309,6 +451,7 @@ static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
>   	if (lane_cfg->clk.pol)
>   		writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(lane_cfg->clk.pos));
> 
> +common:
>   	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));
> @@ -374,6 +517,8 @@ const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
>   	.reg_info = {
>   		.init_seq = lane_regs_x1e80100,
>   		.lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
> +		.cphy_init_seq = lane_regs_x1e80100_cphy,
> +		.cphy_lane_array_size = ARRAY_SIZE(lane_regs_x1e80100_cphy),

haveing "init_seq" and then "cphy_init_seq" is discordant naming.

>   		.common_regs_offset = 0x1000,
>   	},
>   	.supply_names = (const char **)x1e_supplies,
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> index c0a4fa20e..892c3da55 100644
> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> @@ -104,6 +104,19 @@ static int phy_qcom_mipi_csi2_configure(struct phy *phy,
>   	struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
>   	int ret;
> 
> +	if (csi2phy->phy_mode == PHY_MODE_MIPI_CPHY) {
> +		struct phy_configure_opts_mipi_cphy *cphy_cfg = &opts->mipi_cphy;
> +
> +		if (!cphy_cfg->symbol_rate ||
> +		    cphy_cfg->lanes != stream_cfg->num_data_lanes)
> +			return -EINVAL;
> +
> +		/* As for V4L2, a C-PHY link frequency is half the symbol rate. */
> +		stream_cfg->link_freq = cphy_cfg->symbol_rate / 2;

Just use the helper.

> +
> +		return 0;
> +	}
> +
>   	ret = phy_mipi_dphy_config_validate(dphy_cfg);
>   	if (ret)
>   		return ret;
> @@ -124,8 +137,19 @@ static int phy_qcom_mipi_csi2_set_mode(struct phy *phy,
>   {
>   	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> 
> -	if (mode != PHY_MODE_MIPI_DPHY)
> +	/* The mode has to be the bus the endpoint describes. */
> +	switch (mode) {
> +	case PHY_MODE_MIPI_DPHY:
> +		if (csi2phy->bus_type != MEDIA_BUS_TYPE_CSI2_DPHY)
> +			return -EINVAL;
> +		break;
> +	case PHY_MODE_MIPI_CPHY:
> +		if (csi2phy->bus_type != MEDIA_BUS_TYPE_CSI2_CPHY)
> +			return -EINVAL;
> +		break;
> +	default:
>   		return -EOPNOTSUPP;
> +	}
> 
>   	csi2phy->phy_mode = mode;
>   	return 0;
> @@ -258,6 +282,7 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>   	struct fwnode_handle *ep;
>   	int num_polarities;
>   	int num_data_lanes;
> +	int max_lanes;
>   	u32 bus_type;
>   	int i, ret;
> 
> @@ -278,14 +303,25 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>   	bus_type = MEDIA_BUS_TYPE_CSI2_DPHY;
>   	fwnode_property_read_u32(ep, "bus-type", &bus_type);
> 
> -	if (bus_type != MEDIA_BUS_TYPE_CSI2_DPHY) {
> +	switch (bus_type) {
> +	case MEDIA_BUS_TYPE_CSI2_DPHY:
> +		max_lanes = CSI2_MAX_DATA_LANES;
> +		break;
> +	case MEDIA_BUS_TYPE_CSI2_CPHY:
> +		if (csi2phy->soc_cfg->reg_info.cphy_init_seq) {
> +			max_lanes = CSI2_MAX_TRIOS;
> +			break;
> +		}
> +		fallthrough;
> +	default:
>   		ret = -EOPNOTSUPP;
>   		dev_err(dev, "Unsupported bus-type %u\n", bus_type);
>   		goto out_put;
>   	}
> +	csi2phy->bus_type = bus_type;
> 
>   	num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
> -	if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
> +	if (num_data_lanes < 1 || num_data_lanes > max_lanes) {
>   		ret = -EINVAL;
>   		dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
>   		goto out_put;
> @@ -302,6 +338,11 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>   	/* lane-polarities: optional, up to num_data_lanes + 1 entries */
>   	memset(lane_polarities, 0x00, sizeof(lane_polarities));
>   	num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
> +	if (num_polarities > 0 && bus_type == MEDIA_BUS_TYPE_CSI2_CPHY) {
> +		ret = -EINVAL;
> +		dev_err(dev, "lane-polarities do not apply to C-PHY\n");
> +		goto out_put;
> +	}
>   	if (num_polarities > 0) {
>   		if (num_polarities != stream_cfg->num_data_lanes + 1) {
>   			ret = -EINVAL;
> @@ -322,7 +363,7 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>   	csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
> 
>   	for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
> -		if (data_lanes[i] < 1 || data_lanes[i] > CSI2_MAX_DATA_LANES) {
> +		if (data_lanes[i] < 1 || data_lanes[i] > max_lanes) {
>   			dev_err(dev, "Invalid lane %d\n", data_lanes[i]);
>   			ret = -EINVAL;
>   			goto out_put;
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> index 7e55ae007..442c4d6fe 100644
> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> @@ -11,6 +11,7 @@
>   #include <linux/phy/phy.h>
> 
>   #define CSI2_MAX_DATA_LANES	4
> +#define CSI2_MAX_TRIOS		3
>   #define CSI2_DEFAULT_CLK_LANE	7
> 
>   struct mipi_csi2phy_lane {
> @@ -50,6 +51,8 @@ struct mipi_csi2phy_lane_regs {
>   struct mipi_csi2phy_device_regs {
>   	const struct mipi_csi2phy_lane_regs *init_seq;
>   	const int lane_array_size;
> +	const struct mipi_csi2phy_lane_regs *cphy_init_seq;
> +	const int cphy_lane_array_size;
>   	const u32 common_regs_offset;
>   };
> 
> @@ -75,6 +78,7 @@ struct mipi_csi2phy_soc_cfg {
>   struct mipi_csi2phy_device {
>   	struct device *dev;
>   	u8 phy_mode;
> +	u32 bus_type;
> 
>   	struct phy *phy;
>   	void __iomem *base;
> 
> --
> 2.53.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
  2026-10-15 22:20 ` [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100 George Neacsu via B4 Relay
  2026-10-16  9:29   ` Bryan O'Donoghue
@ 2026-10-16  9:59   ` Nihal Kumar Gupta
  2026-10-16 10:06     ` George Neacsu
  2026-10-16 10:23   ` Konrad Dybcio
  2 siblings, 1 reply; 17+ messages in thread
From: Nihal Kumar Gupta @ 2026-10-16  9:59 UTC (permalink / raw)
  To: geocausa, Bryan O'Donoghue, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel, Vikram Sharma,
	Suresh Vankadara, tjose



On 16-10-2026 03:50, George Neacsu via B4 Relay wrote:
> From: George Neacsu <geocausa@gmail.com>
> 
> The CSI2 PHY schema already allows bus-type C-PHY, but the driver
> rejects it. Accept it on SoCs that provide a C-PHY sequence: up to three
> trios and no lane polarities, and set_mode() has to ask for the mode the
> endpoint describes. In C-PHY mode, configure() takes the symbol rate.
> lanes_enable() powers up the trios on the odd CTRL5 bits and writes the
> C-PHY sequence, which carries its own timing, so no settle count is
> computed.
> 
> The X1E80100 sequence is Qualcomm's for the v2.1.2 3PH PHY at 2.5 Gsps,
> taken from the downstream camera driver's GPL-2.0
> cam_csiphy_2_1_2_hwreg.h (csiphy_3ph_v2_1_2_reg, datarate_212_2p5Gsps
> and the 3PH reset exit), as the D-PHY sequence above it is.
> 
> Tested on the Surface Pro 11 (X1E80100) with an IMX681 on CSIPHY2, one
> trio at 2.4 Gsym/s, and raw capture through CSID1 and an RDI interface:
> 3844x2640 at 30.01 fps, and the sensor's colour bars are byte-identical
> to the same pattern captured with an out-of-tree driver. D-PHY is
> unchanged: an OV13858 on CSIPHY1 over four lanes streams at 29.9 fps.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
Hi George,

Thanks for working on this. I also have local C-PHY enablement patches, so it
is good to see this being discussed upstream.

The overall direction looks reasonable to me, especially adding a generic
PHY_MODE_MIPI_CPHY and letting CAMSS request C-PHY through the PHY API.

> ---
>  drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 151 ++++++++++++++++++++-
>  drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     |  49 ++++++-
>  drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |   4 +
>  3 files changed, 197 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> index 966d79c98..e758b6755 100644
> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> @@ -6,6 +6,7 @@
>   * Copyright (C) 2016-2026 Linaro Ltd.
>   */
>  
> +#include <dt-bindings/media/video-interfaces.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -161,6 +162,131 @@ mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
>  	{.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
>  };
>
> +/*
> + * 4nm 3PH v2.1.2 C-PHY, 2.5 Gsps, from Qualcomm's GPL-2.0 camera driver
> + * (cam_csiphy_2_1_2_hwreg.h: csiphy_3ph_v2_1_2_reg[], datarate_212_2p5Gsps[]
> + * and the 3PH reset-exit write), one trio block per lane triplet.
> + */
> +static const struct
> +mipi_csi2phy_lane_regs lane_regs_x1e80100_cphy[] = {
This table is C-PHY specific, but it is being added to
phy-qcom-mipi-csi2-3ph-dphy.c

Bryan, do you have any preference on how to organize this? Should we keep both
D-PHY and C-PHY sequences here for now, or move the larger C-PHY register
tables to a separate hwreg/table file?
> +	/* Power up lanes 3ph mode */
> +	{.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* datarate_212_2p5Gsps: AFE settings */
> +	{.reg_addr = 0x0268, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0294, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},

> +	{.reg_addr = 0x0668, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0694, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
...
> +	{.reg_addr = 0x0a68, .reg_data = 0xf1, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a94, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
...> +	/* csiphy_3ph_v2_1_2_reg */
> +	{.reg_addr = 0x02f4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
...> +	{.reg_addr = 0x06f4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x06f8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
...
> +	{.reg_addr = 0x0af4, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0af8, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
...> +	/* datarate_212_2p5Gsps, then csiphy_3ph_v2_1_2_reg */
> +	{.reg_addr = 0x020c, .reg_data = 0x22, .param_type = CSIPHY_DEFAULT_PARAMS},

Could we add blank lines between the repeated per-trio register blocks here?

The table would be easier to review if the `0x02*`, `0x06*`, and `0x0a*`
blocks were visually separated, similar to how the D-PHY table separates the
per-lane register groups. It would also make comparison with the downstream
source simpler.

> +	{.reg_addr = 0x0a84, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	{.reg_addr = 0x0a90, .reg_data = 0x02, .param_type = CSIPHY_DEFAULT_PARAMS},
> +	/* csiphy_reset_exit_reg_2_1_2, 3PH */
> +	{.reg_addr = 0x1000, .reg_data = 0x0e, .delay_us = 3048,
Any specific reason for this much delay?

> +	 .param_type = CSIPHY_DEFAULT_PARAMS},
> +};
> +
For this C-PHY table, I do not see a CSIPHY_SETTLE_CNT_LOWER_BYTE entry.
Is the settle timing already encoded in one of the fixed register values in
lane_regs_x1e80100_cphy[]?

If yes, which register corresponds to the C-PHY settle/timing value?

Also, is that value specific to the 2.5 Gsps table? My understanding is that
the settle count calculation would be data-rate specific, based on the C-PHY
symbol rate per trio.

>  static inline const struct mipi_csi2phy_device_regs *
>  csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
>  {
> @@ -246,8 +372,11 @@ 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;
> +	const bool cphy = csi2phy->bus_type == MEDIA_BUS_TYPE_CSI2_CPHY;
> +	const struct mipi_csi2phy_lane_regs *r = cphy ? regs->cphy_init_seq :
> +								regs->init_seq;
> +	int i, array_size = cphy ? regs->cphy_lane_array_size :
> +				   regs->lane_array_size;
>  	u32 val;
>  
>  	for (i = 0; i < array_size; i++, r++) {
> @@ -273,13 +402,26 @@ static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
>  {
>  	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;
> +	const bool cphy = csi2phy->bus_type == MEDIA_BUS_TYPE_CSI2_CPHY;
Could we avoid carrying a separate parsed `bus_type` and derive the PHY type from 
the PHY layer state instead?
> +	u8 settle_cnt = 0;
>  	u8 val;
>  	int i;
>  
>  	if (cfg->link_freq <= 0)
>  		return -EINVAL;
>  
> +	if (cphy) {
> +		/* The C-PHY sequence carries its own timing; trios are the odd bits. */
> +		val = 0;
> +		for (i = 0; i < cfg->num_data_lanes; i++)
> +			val |= BIT(lane_cfg->data[i].pos * 2 + 1);
> +
> +		writel(val, csi2phy->base +
> +		       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> +
> +		goto common;
> +	}
The C-PHY and D-PHY lane-mask calculation could be combined into a helper to
avoid open-coding the C-PHY path separately.

Something like this:
static u8 csiphy_get_lane_mask(struct mipi_csi2phy_lanes_cfg *lane_cfg)
{
	u8 lane_mask = 0;
	u8 offset = 0;

	switch (lane_cfg->phy_cfg) {
		case V4L2_MBUS_CSI2_CPHY:
			offset = 1;
			break;
		case V4L2_MBUS_CSI2_DPHY:
			lane_mask = BIT(lane_cfg->clk.pos);
			break;
		default:
			break;
	}

	/*
	 * CSI_COMMON_CTRL5 is a physical lane power-up bitmap:
	 * - Bits [0,2,4,6] → D-PHY data lanes(LN0, LN2, LN4, LN6)
	 * - Bits [1,3,5] → C-PHY trio lanes(LN1, LN3, LN5)
	 * - Bit [7] → D-PHY clock lane(LNCK) dedicated clock enable
	 */
	for (int i = 0; i < lane_cfg->num_data; i++)
		lane_mask |= BIT((lane_cfg->data[i].pos * 2) + offset);

	return lane_mask;
}

> +
>  	settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
>  	if (!settle_cnt)
>  		return -ENODEV;
> @@ -309,6 +451,7 @@ static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
>  	if (lane_cfg->clk.pol)
>  		writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(lane_cfg->clk.pos));
>  
> +common:
>  	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));
> @@ -374,6 +517,8 @@ const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
>  	.reg_info = {
>  		.init_seq = lane_regs_x1e80100,
>  		.lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
> +		.cphy_init_seq = lane_regs_x1e80100_cphy,
> +		.cphy_lane_array_size = ARRAY_SIZE(lane_regs_x1e80100_cphy),
>  		.common_regs_offset = 0x1000,
>  	},
>  	.supply_names = (const char **)x1e_supplies,
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> index c0a4fa20e..892c3da55 100644
> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> @@ -104,6 +104,19 @@ static int phy_qcom_mipi_csi2_configure(struct phy *phy,
>  	struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
>  	int ret;
>  
> +	if (csi2phy->phy_mode == PHY_MODE_MIPI_CPHY) {
> +		struct phy_configure_opts_mipi_cphy *cphy_cfg = &opts->mipi_cphy;
> +
> +		if (!cphy_cfg->symbol_rate ||
> +		    cphy_cfg->lanes != stream_cfg->num_data_lanes)
> +			return -EINVAL;
> +
> +		/* As for V4L2, a C-PHY link frequency is half the symbol rate. */
> +		stream_cfg->link_freq = cphy_cfg->symbol_rate / 2;
> +
> +		return 0;
> +	}
> +
>  	ret = phy_mipi_dphy_config_validate(dphy_cfg);
>  	if (ret)
>  		return ret;
> @@ -124,8 +137,19 @@ static int phy_qcom_mipi_csi2_set_mode(struct phy *phy,
>  {
>  	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
>  
> -	if (mode != PHY_MODE_MIPI_DPHY)
> +	/* The mode has to be the bus the endpoint describes. */
> +	switch (mode) {
> +	case PHY_MODE_MIPI_DPHY:
> +		if (csi2phy->bus_type != MEDIA_BUS_TYPE_CSI2_DPHY)
> +			return -EINVAL;
> +		break;
> +	case PHY_MODE_MIPI_CPHY:
> +		if (csi2phy->bus_type != MEDIA_BUS_TYPE_CSI2_CPHY)
> +			return -EINVAL;
> +		break;
> +	default:
>  		return -EOPNOTSUPP;
> +	}
>  
>  	csi2phy->phy_mode = mode;
>  	return 0;
> @@ -258,6 +282,7 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>  	struct fwnode_handle *ep;
>  	int num_polarities;
>  	int num_data_lanes;
> +	int max_lanes;
>  	u32 bus_type;
>  	int i, ret;
>  
> @@ -278,14 +303,25 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>  	bus_type = MEDIA_BUS_TYPE_CSI2_DPHY;
>  	fwnode_property_read_u32(ep, "bus-type", &bus_type);
>  
> -	if (bus_type != MEDIA_BUS_TYPE_CSI2_DPHY) {
> +	switch (bus_type) {
> +	case MEDIA_BUS_TYPE_CSI2_DPHY:
> +		max_lanes = CSI2_MAX_DATA_LANES;
> +		break;
> +	case MEDIA_BUS_TYPE_CSI2_CPHY:
> +		if (csi2phy->soc_cfg->reg_info.cphy_init_seq) {
> +			max_lanes = CSI2_MAX_TRIOS;
> +			break;
> +		}
> +		fallthrough;
> +	default:
>  		ret = -EOPNOTSUPP;
>  		dev_err(dev, "Unsupported bus-type %u\n", bus_type);
>  		goto out_put;
>  	}
> +	csi2phy->bus_type = bus_type;
>  
>  	num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
> -	if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
> +	if (num_data_lanes < 1 || num_data_lanes > max_lanes) {
>  		ret = -EINVAL;
>  		dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
>  		goto out_put;
> @@ -302,6 +338,11 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>  	/* lane-polarities: optional, up to num_data_lanes + 1 entries */
>  	memset(lane_polarities, 0x00, sizeof(lane_polarities));
>  	num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
> +	if (num_polarities > 0 && bus_type == MEDIA_BUS_TYPE_CSI2_CPHY) {
> +		ret = -EINVAL;
> +		dev_err(dev, "lane-polarities do not apply to C-PHY\n");
> +		goto out_put;
> +	}
>  	if (num_polarities > 0) {
>  		if (num_polarities != stream_cfg->num_data_lanes + 1) {
>  			ret = -EINVAL;
> @@ -322,7 +363,7 @@ static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
>  	csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
>  
>  	for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
> -		if (data_lanes[i] < 1 || data_lanes[i] > CSI2_MAX_DATA_LANES) {
> +		if (data_lanes[i] < 1 || data_lanes[i] > max_lanes) {
>  			dev_err(dev, "Invalid lane %d\n", data_lanes[i]);
>  			ret = -EINVAL;
>  			goto out_put;
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> index 7e55ae007..442c4d6fe 100644
> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
> @@ -11,6 +11,7 @@
>  #include <linux/phy/phy.h>
>  
>  #define CSI2_MAX_DATA_LANES	4
> +#define CSI2_MAX_TRIOS		3
>  #define CSI2_DEFAULT_CLK_LANE	7
>  
>  struct mipi_csi2phy_lane {
> @@ -50,6 +51,8 @@ struct mipi_csi2phy_lane_regs {
>  struct mipi_csi2phy_device_regs {
>  	const struct mipi_csi2phy_lane_regs *init_seq;
>  	const int lane_array_size;
> +	const struct mipi_csi2phy_lane_regs *cphy_init_seq;
> +	const int cphy_lane_array_size;
This gives us a single C-PHY register sequence for the SoC. Do we expect this
to be enough long term?

The X1E80100 table being added here is for 2.5 Gsps, but C-PHY timing/settings
can be data-rate specific. Should we extend this structure now to support
multiple C-PHY rate settings, similar to selecting a table by symbol rate, or
is it better to keep this simple for this series and handle per-rate tables
as a follow-up?

Bryan, do you have a preference on that?

>  	const u32 common_regs_offset;
>  };
>  
> @@ -75,6 +78,7 @@ struct mipi_csi2phy_soc_cfg {
>  struct mipi_csi2phy_device {
>  	struct device *dev;
>  	u8 phy_mode;
> +	u32 bus_type;
>  
>  	struct phy *phy;
>  	void __iomem *base;
> 

-- 
Regards,
Nihal Kumar Gupta


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
  2026-10-16  9:29   ` Bryan O'Donoghue
@ 2026-10-16 10:06     ` George Neacsu
  2026-10-16 10:12       ` Bryan O'Donoghue
  0 siblings, 1 reply; 17+ messages in thread
From: George Neacsu @ 2026-10-16 10:06 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Vinod Koul, David Heidelberg, Neil Armstrong, Vladimir Zapolskiy,
	Loic Poulain, Mauro Carvalho Chehab, Manivannan Sadhasivam,
	linux-phy, linux-media, linux-arm-msm, linux-kernel

Hi Bryan,

Thanks for the reviews, and thanks to Loic as well.

The series is on next-20261015. There, phy-qcom-mipi-csi2-3ph-dphy.c is
blob 966d79c98, the same as on the media-committers next branch, so I
think your tree has a different revision of the CSI2 PHY driver. 3/4 and
4/4 also sit on top of David's "C-PHY configuration on Qualcomm
platforms" v9 [1], for phy_sel and the C-PHY link frequency, which is not
in -next yet.

Which base would you like v2 on? If your branch is public I will rebase
onto it. Otherwise I will use -next plus David's series and spell the
dependency out in the cover letter.

For v2 I have:

- 1/4: lanes -> trios, and a PHY-layer helper for the C-PHY symbol rate,
  the analogue of phy_mipi_dphy_get_default_config_for_hsclk()
- 2/4: dphy_/cphy_ naming for the init sequences, if/else instead of the
  ternaries, the D-PHY and C-PHY lane setup split into helpers instead of
  the goto, and the helper in configure()
- 3/4: FIELD_PREP() with a mask for the PHY type select
- 4/4: the helper, and an enum phy_mode variable passed straight to
  phy_set_mode()

[1] https://lore.kernel.org/all/20260617-qcom-cphy-v9-0-83da8a8e4e44@ixit.cz/

Thanks,
George

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
  2026-10-16  9:59   ` Nihal Kumar Gupta
@ 2026-10-16 10:06     ` George Neacsu
  0 siblings, 0 replies; 17+ messages in thread
From: George Neacsu @ 2026-10-16 10:06 UTC (permalink / raw)
  To: Nihal Kumar Gupta
  Cc: Bryan O'Donoghue, Vinod Koul, David Heidelberg,
	Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel, Vikram Sharma,
	Suresh Vankadara, tjose

Hi Nihal,

On 16/10/2026 15:29, Nihal Kumar Gupta wrote:
> Thanks for working on this. I also have local C-PHY enablement patches, so it
> is good to see this being discussed upstream.

Thanks for the review. If your patches cover more SoCs or data rates, I am
happy to fold them in or to rebase on top of them, whichever is easier for
you. I can test anything on the Surface Pro 11 (X1E80100, IMX681 on
CSIPHY2, one trio).

> Could we add blank lines between the repeated per-trio register blocks here?

Yes, will do in v2.

>> +	{.reg_addr = 0x1000, .reg_data = 0x0e, .delay_us = 3048,
> Any specific reason for this much delay?

It is copied from the downstream reset exit entry,
{0x1000, 0x0E, 0xBE8, CSIPHY_3PH_REGS}, and I have not tried anything
shorter. If you know the real requirement I will use it. Either way, a
delay this long should be a sleep rather than udelay(), which v2 will fix.

> For this C-PHY table, I do not see a CSIPHY_SETTLE_CNT_LOWER_BYTE entry.
> Is the settle timing already encoded in one of the fixed register values in
> lane_regs_x1e80100_cphy[]?

Yes: 0x020c/0x0208 per trio (0x060c/0x0608, 0x0a0c/0x0a08 for the other
two) are the settle count low and high bytes. Downstream marks them
CSIPHY_SETTLE_CNT_LOWER_BYTE/HIGHER_BYTE and overwrites them with the
count derived from the sensor settle time. The RFC writes the table's
default, 0x22/0x00, as fixed values. That works for the IMX681 at
2.4 Gsym/s, but you are right that it is rate dependent. In v2 these
entries will use the settle count parameter types and the count will be
computed from the symbol rate, as for D-PHY.

> Could we avoid carrying a separate parsed `bus_type` and derive the PHY type
> from the PHY layer state instead?

Agreed. The bus type is still needed at parse time for the lane count and
polarity checks, but the runtime paths will use phy_mode.

> The C-PHY and D-PHY lane-mask calculation could be combined into a helper to
> avoid open-coding the C-PHY path separately.

Thanks, I will take your helper. Together with Bryan's request, this also
removes the goto.

> This gives us a single C-PHY register sequence for the SoC. Do we expect this
> to be enough long term?

Not long term, no. I only have one C-PHY sensor running at one rate, so I
would prefer to keep v2 to the single table with the settle count fixed,
and add selection by symbol rate as a follow-up, unless Bryan would rather
have the structure in place now.

Thanks,
George

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
  2026-10-16 10:06     ` George Neacsu
@ 2026-10-16 10:12       ` Bryan O'Donoghue
  0 siblings, 0 replies; 17+ messages in thread
From: Bryan O'Donoghue @ 2026-10-16 10:12 UTC (permalink / raw)
  To: George Neacsu
  Cc: Vinod Koul, David Heidelberg, Neil Armstrong, Vladimir Zapolskiy,
	Loic Poulain, Mauro Carvalho Chehab, Manivannan Sadhasivam,
	linux-phy, linux-media, linux-arm-msm, linux-kernel

On 16/10/2026 11:06, George Neacsu wrote:
> Hi Bryan,
> 
> Thanks for the reviews, and thanks to Loic as well.
> 
> The series is on next-20261015. There, phy-qcom-mipi-csi2-3ph-dphy.c is
> blob 966d79c98, the same as on the media-committers next branch
Series doesn't apply to linux-next or phy-next - given we are at -rc7 
linux-next is probably better, after -rc7 phy-next is best.

Either -next:

[~/Development/worktree/bod-media-committers]$ git fetch linux-next 
  
                              *[bod-qcom-media-committers]
[~/Development/worktree/bod-media-committers]$ git reset --hard 
linux-next/master 
                                      *[bod-qcom-media-committers]
HEAD is now at b810335a0699d Add linux-next specific files for 20261015
[~/Development/worktree/bod-media-committers]$ b4 shazam 
20261015-x1e-cphy-rfc-v1-0-1f3fa9af940a@gmail.com 
                                             *[bod-qcom-media-committers]
Looking up 
https://lore.kernel.org/all/20261015-x1e-cphy-rfc-v1-0-1f3fa9af940a@gmail.com/
Grabbing thread from 
lore.kernel.org/all/20261015-x1e-cphy-rfc-v1-0-1f3fa9af940a@gmail.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 15 messages in the thread
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
   ✗ [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options
     + Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> (✓ 
DKIM/linaro.org)
   ✗ [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
   ✗ [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the 
receiver
     + Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com> (✓ 
DKIM/oss.qualcomm.com)
   ✗ [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API
   ---
   ✗ No key: ed25519/geocausa@gmail.com
---
Total patches: 4
---
Applying: phy: Add MIPI C-PHY mode and configuration options
Applying: phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
Applying: media: qcom: camss: csid-680: Select C-PHY in the receiver
Applying: media: qcom: camss: Configure C-PHY through the PHY API
Patch failed at 0004 media: qcom: camss: Configure C-PHY through the PHY API
error: patch failed: drivers/media/platform/qcom/camss/camss-csiphy.c:329
error: drivers/media/platform/qcom/camss/camss-csiphy.c: patch does not 
apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am 
--abort".
hint: Disable this message with "git config set advice.mergeConflict false"

Or -phy:

[~/Development/worktree/bod-media-committers]$ git fetch phy 
  
                              *[bod-qcom-media-committers]
 From git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy
    c7f2322431cb6..8f56fdf3d8cb9  next       -> phy/next
[~/Development/worktree/bod-media-committers]$ git reset --hard phy/next 
  
                              *[bod-qcom-media-committers]
Updating files: 100% (16814/16814), done.
HEAD is now at 8f56fdf3d8cb9 phy: tegra: Add support for Tegra238 XUSB 
pad controller
[~/Development/worktree/bod-media-committers]$ git am --abort 
  
                              *[bod-qcom-media-committers]
warning: You seem to have moved HEAD since the last 'am' failure.
Not rewinding to ORIG_HEAD
[~/Development/worktree/bod-media-committers]$ b4 shazam 
20261015-x1e-cphy-rfc-v1-0-1f3fa9af940a@gmail.com 
                                             *[bod-qcom-media-committers]
Looking up 
https://lore.kernel.org/all/20261015-x1e-cphy-rfc-v1-0-1f3fa9af940a@gmail.com/
Grabbing thread from 
lore.kernel.org/all/20261015-x1e-cphy-rfc-v1-0-1f3fa9af940a@gmail.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 15 messages in the thread
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
   ✗ [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options
     + Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> (✓ 
DKIM/linaro.org)
   ✗ [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
   ✗ [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the 
receiver
     + Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com> (✓ 
DKIM/oss.qualcomm.com)
   ✗ [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API
   ---
   ✗ No key: ed25519/geocausa@gmail.com
---
Total patches: 4
---
Applying: phy: Add MIPI C-PHY mode and configuration options
Applying: phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
Applying: media: qcom: camss: csid-680: Select C-PHY in the receiver
Applying: media: qcom: camss: Configure C-PHY through the PHY API
Patch failed at 0004 media: qcom: camss: Configure C-PHY through the PHY API
error: patch failed: drivers/media/platform/qcom/camss/camss-csiphy.c:329
error: drivers/media/platform/qcom/camss/camss-csiphy.c: patch does not 
apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am 
--abort".
hint: Disable this message with "git config set advice.mergeConflict false"

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100
  2026-10-15 22:20 ` [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100 George Neacsu via B4 Relay
  2026-10-16  9:29   ` Bryan O'Donoghue
  2026-10-16  9:59   ` Nihal Kumar Gupta
@ 2026-10-16 10:23   ` Konrad Dybcio
  2 siblings, 0 replies; 17+ messages in thread
From: Konrad Dybcio @ 2026-10-16 10:23 UTC (permalink / raw)
  To: geocausa, Bryan O'Donoghue, Vinod Koul, David Heidelberg
  Cc: Neil Armstrong, Vladimir Zapolskiy, Loic Poulain,
	Mauro Carvalho Chehab, Manivannan Sadhasivam, linux-phy,
	linux-media, linux-arm-msm, linux-kernel

On 10/16/26 12:20 AM, George Neacsu via B4 Relay wrote:
> From: George Neacsu <geocausa@gmail.com>
> 
> The CSI2 PHY schema already allows bus-type C-PHY, but the driver
> rejects it. Accept it on SoCs that provide a C-PHY sequence: up to three
> trios and no lane polarities, and set_mode() has to ask for the mode the
> endpoint describes. In C-PHY mode, configure() takes the symbol rate.
> lanes_enable() powers up the trios on the odd CTRL5 bits and writes the
> C-PHY sequence, which carries its own timing, so no settle count is

[...]


> +/*
> + * 4nm 3PH v2.1.2 C-PHY, 2.5 Gsps, from Qualcomm's GPL-2.0 camera driver
> + * (cam_csiphy_2_1_2_hwreg.h: csiphy_3ph_v2_1_2_reg[], datarate_212_2p5Gsps[]
> + * and the 3PH reset-exit write), one trio block per lane triplet.
> + */
> +static const struct
> +mipi_csi2phy_lane_regs lane_regs_x1e80100_cphy[] = {

Let's rename the struct to make this comment redundant.

FWIW 3ph === CPHY, 2ph === DPHY, so we can drop that. Maybe
just lane_regs_v2_1_2_cphy[]?

Konrad



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-10-16 10:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 22:20 [PATCH RFC 0/4] phy: qcom-mipi-csi2: C-PHY for X1E80100 George Neacsu via B4 Relay
2026-10-15 22:20 ` [PATCH RFC 1/4] phy: Add MIPI C-PHY mode and configuration options George Neacsu via B4 Relay
2026-10-16  9:13   ` Bryan O'Donoghue
2026-10-16  9:24   ` Loic Poulain
2026-10-15 22:20 ` [PATCH RFC 2/4] phy: qcom-mipi-csi2: Add C-PHY support for X1E80100 George Neacsu via B4 Relay
2026-10-16  9:29   ` Bryan O'Donoghue
2026-10-16 10:06     ` George Neacsu
2026-10-16 10:12       ` Bryan O'Donoghue
2026-10-16  9:59   ` Nihal Kumar Gupta
2026-10-16 10:06     ` George Neacsu
2026-10-16 10:23   ` Konrad Dybcio
2026-10-15 22:20 ` [PATCH RFC 3/4] media: qcom: camss: csid-680: Select C-PHY in the receiver George Neacsu via B4 Relay
2026-10-16  7:50   ` Loic Poulain
2026-10-15 22:20 ` [PATCH RFC 4/4] media: qcom: camss: Configure C-PHY through the PHY API George Neacsu via B4 Relay
2026-10-16  7:56   ` Loic Poulain
2026-10-16  8:59   ` Bryan O'Donoghue
2026-10-16  9:09   ` Bryan O'Donoghue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox