Re: [PATCH 4/6] media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY

From: Hangxiang Ma

Date: Tue Oct 14 2025 - 23:41:11 EST



On 9/25/2025 8:57 PM, Bryan O'Donoghue wrote:

On 25/09/2025 01:02, Jingyi Wang wrote:
From: Hangxiang Ma <hangxiang.ma@xxxxxxxxxxxxxxxx>

Add more detailed resource information for CSIPHY devices in the camss
driver along with the support for v2.4.0 in the 2 phase CSIPHY driver
that is responsible for the PHY lane register configuration, module
reset and interrupt handling.

This change adds 'cmn_status_offset' variable in 'csidphy_device_regs'
structure. It helps adapt the offset to the common status registers that
is different in v2.4.0 from others.

Signed-off-by: Hangxiang Ma <hangxiang.ma@xxxxxxxxxxxxxxxx>
Signed-off-by: Jingyi Wang <jingyi.wang@xxxxxxxxxxxxxxxx>
---
  .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 138 ++++++++++++++++++++-
  drivers/media/platform/qcom/camss/camss-csiphy.h   |   1 +
  drivers/media/platform/qcom/camss/camss.c          | 107 ++++++++++++++++
  3 files changed, 240 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
index a229ba04b158..ecb91d3688ca 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
@@ -46,7 +46,7 @@
  #define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE    BIT(7)
  #define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B BIT(0)
  #define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID    BIT(1)
-#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n) ((offset) + 0xb0 + 0x4 * (n))
+#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, bias, n) ((offset) + (bias) + 0x4 * (n))

You need to explain this bias parameter in the commit log.

Ack. Now rename the 'bias' parameter the same as 'common_status_offset' to remove ambiguity.


  #define CSIPHY_DEFAULT_PARAMS        0
  #define CSIPHY_LANE_ENABLE        1
@@ -587,6 +587,123 @@ csiphy_lane_regs lane_regs_sm8550[] = {
      {0x0C64, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
  };

+/* GEN2 2.4.0 2PH DPHY mode */

You need to call out the process node in this comment, per the other recent additions.

Ack

+static const struct
+csiphy_lane_regs lane_regs_kaanapali[] = {
+    /* LN 0 */
+    {0x0094, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x00A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0090, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0098, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0094, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
+    {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0000, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0038, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0034, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x001C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0014, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x003C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0020, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0008, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+    {0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0094, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+    {0x005C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+    {0x0060, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+    {0x0064, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+    /* LN 2 */
+    {0x0494, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x04A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0490, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0498, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0494, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
+    {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0400, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0438, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x042C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0434, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x041C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0414, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x043C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0404, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0420, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0408, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+    {0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0494, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+    {0x045C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+    {0x0460, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+    {0x0464, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+    /* LN 4 */
+    {0x0894, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x08A0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0890, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0898, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0894, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
+    {0x0830, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0800, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0838, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x082C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0834, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x081C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0814, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x083C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0804, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0820, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0808, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+    {0x0810, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0894, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+    {0x085C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+    {0x0860, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+    {0x0864, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+    /* LN 6 */
+    {0x0C94, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0CA0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C90, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C94, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
+    {0x0C30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C00, 0x8C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C38, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C2C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C34, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C1C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C14, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C3C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C04, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C20, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C08, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+    {0x0C10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0C94, 0xD7, 0x00, CSIPHY_SKEW_CAL},
+    {0x0C5C, 0x54, 0x00, CSIPHY_SKEW_CAL},
+    {0x0C60, 0xFD, 0x00, CSIPHY_SKEW_CAL},
+    {0x0C64, 0x7F, 0x00, CSIPHY_SKEW_CAL},
+
+    /* LN CLK */
+    {0x0E94, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0EA0, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E90, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E98, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E94, 0x07, 0xd1, CSIPHY_DEFAULT_PARAMS},
+    {0x0E30, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E28, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E00, 0x80, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E0C, 0xFF, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E38, 0x1F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E2C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E34, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E1C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E14, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E3C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E04, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E20, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+    {0x0E08, 0x19, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+    {0x0E10, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
+};
+
  /* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
  static const struct
  csiphy_lane_regs lane_regs_x1e80100[] = {
@@ -714,13 +831,13 @@ static void csiphy_hw_version_read(struct csiphy_device *csiphy,
             CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 6));

      hw_version = readl_relaxed(csiphy->base +
- CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 12));
+        CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, regs->cmn_status_offset, 12));
      hw_version |= readl_relaxed(csiphy->base +
- CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 13)) << 8;
+        CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, regs->cmn_status_offset, 13)) << 8;
      hw_version |= readl_relaxed(csiphy->base +
- CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 14)) << 16;
+        CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, regs->cmn_status_offset, 14)) << 16;
      hw_version |= readl_relaxed(csiphy->base +
- CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, 15)) << 24;
+        CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, regs->cmn_status_offset, 15)) << 24;

      dev_dbg(dev, "CSIPHY 3PH HW Version = 0x%08x\n", hw_version);
  }
@@ -749,7 +866,8 @@ static irqreturn_t csiphy_isr(int irq, void *dev)
      for (i = 0; i < 11; i++) {
          int c = i + 22;
          u8 val = readl_relaxed(csiphy->base +
- CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset, i));
+ CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->offset,
+                              regs->cmn_status_offset, i));

          writel_relaxed(val, csiphy->base +
CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, c));
@@ -915,6 +1033,7 @@ static bool csiphy_is_gen2(u32 version)
      case CAMSS_845:
      case CAMSS_8550:
      case CAMSS_8775P:
+    case CAMSS_KAANAPALI:
      case CAMSS_X1E80100:
          ret = true;
          break;
@@ -989,6 +1108,7 @@ static int csiphy_init(struct csiphy_device *csiphy)

      csiphy->regs = regs;
      regs->offset = 0x800;
+    regs->cmn_status_offset = 0xb0;

      switch (csiphy->camss->res->version) {
      case CAMSS_845:
@@ -1023,6 +1143,12 @@ static int csiphy_init(struct csiphy_device *csiphy)
          regs->lane_regs = &lane_regs_sa8775p[0];
          regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
          break;
+    case CAMSS_KAANAPALI:
+        regs->lane_regs = &lane_regs_kaanapali[0];
+        regs->lane_array_size = ARRAY_SIZE(lane_regs_kaanapali);
+        regs->offset = 0x1000;
+        regs->cmn_status_offset = 0x138;

I don't think a second offset is warranted

You could acheive the required offset with offset = 0x1138; and a comment.

Perhaps I'm not seeing it but seems like an additional - fixed - fluff variable.
Necessary to add another variable here. The 'offset' parameter denotes the address offset between base and the common register. But the 'common_status_offset' denotes the offset between common registers and status registers.