* [PATCH v7 0/8] media: camss: Add support for C-PHY configuration on Qualcomm platforms
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
Note: WIP tag added, as not everything from the previous review round has
been addressed.
# Short summary
This patch series extends the Qualcomm CAMSS (Camera Subsystem),
including CSID and CSIPHY components, to support C-PHY mode configuration.
# Background and motivation
Modern smartphone cameras increasingly rely on MIPI C-PHY rather than
D-PHY, thanks to its higher data throughput and signal efficiency.
As a result, many OEMs adopt C-PHY interfaces for main (rear) cameras on
Qualcomm-based devices.
Until now, mainline Linux lacked C-PHY configuration support for Qualcomm
chipsets, preventing bring-up of primary camera sensors on several
Snapdragon platforms. This series closes that gap.
- Introduces C-PHY configuration support for the CAMSS driver stack,
covering both CSID and CSIPHY blocks.
- Successfully enables C-PHY operation on the Snapdragon 845 platform.
- Tested on OnePlus 6 and 6T phones running mainline Linux,
using the Sony IMX519 main camera sensor.
- The new configuration allows other chipsets versionsto enable C-PHY by
simply adding corresponding sensor driver support and csiphy
initialization data, following the example set for sdm845.
With this patch series, mainline Linux gains working C-PHY support for
Snapdragon 845, paving the way for improved main camera functionality
across many Qualcomm-based devices. The groundwork also simplifies
future enablement efforts for additional SoCs and sensors.
Until merged, the series will be also available at:
https://codeberg.org/sdm845/linux/commits/branch/b4/qcom-cphy
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Changes in v7:
- Move logic out of switch. (Frank)
- Change csid cphy knob to phy_sel using enum. (Bryan, Frank)
- Link to v6: https://lore.kernel.org/r/20260603-qcom-cphy-v6-0-e50de0b557a8@ixit.cz
Changes in v6:
- lanes_enable now uses WARN_ONCE instead of return values. (Sakari)
- Dropped Kodiak patch until it gets better documented.
- Document the 3ph C-PHY sdm845 registers a bit.
- Link to v5: https://lore.kernel.org/r/20260531-qcom-cphy-v5-0-6be0f62b4d65@ixit.cz
Changes in v5:
- Split first patch enabling C-PHY part into last patch. (Bryan)
- Fix the camss_get_link_freq crash. (Cory)
- Dropped patch whitelisting C-PHY, instead use nullptr check. (Bryan)
- Dropped A-b/R-b from
"media: qcom: camss: Initialize lanes after lane configuration is available"
and changed Petr to Co-developed by and owned the patch.
- Link to v4: https://lore.kernel.org/r/20260301-qcom-cphy-v4-0-e53316d2cc65@ixit.cz
Changes in v4:
- Documented cphy parametr to camss_get_link_freq.
- Use BIT() macro for lane_mask. (Bryan)
- Correct lane_mask calculation. (Kieran + me)
- Removed comment for the D/C-PHY sequences init. (Bryan)
- Pass &csid->phy for calculate freq. (Bryan)
- Added missing cphy description to camss_get_link_freq. (kernel test robot)
- Gen2 v1.1 MIPI CSI-2 CPHY init hex to lowercase.
- Added back missed commit with improved electrical for sdm845 3ph.
- NOT addressed yet:
- Proliferating special cases in switch statements on a per-SoC basis is verboten.
- is it possible to set clock_lane to say 0xff in DT ?
- Link to v3: https://lore.kernel.org/r/20260117-qcom-cphy-v3-0-8ce76a06f7db@ixit.cz
Changes in v3:
- Make lanes_enable return sucess or error, since I couldn't move the
configuration to the _init.
- Dropped R-b tags on
"media: qcom: camss: Initialize lanes after lane configuration is available"
as I changed formatting.
- Link to v2: https://lore.kernel.org/r/20251204-qcom-cphy-v2-0-6b35ef8b071e@ixit.cz
Changes in v2:
- This is still WIP patch series, thus I wanted to publish already
changed parts to get feedback regarding to the direction of patchset.
- When switch to using odd bits, zeroed val which was left unitialized in v1.
- Accidentally missed archs added back in the commit moving lane regs to
new location.
- Remove commit with reverting check for only D-PHY is supported and
adjusted the check to also account for C-PHY.
- Documented link frequency calculation with defines. (Casey)
- Changed the cphy boolean to phy_cfg enum in the camss/camss-csiphy.
(Brian)
- Added patch for csiphy-3ph enablement for sm7280 from Luca as I'm
meanwhile trying to bring up the C-PHY sensor on FairPhone 5.
- Merged these two commits together
csiphy-3ph: Enable sdm845 C-PHY sequence
csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 CPHY init
merged R-b.
- Link to v1: https://lore.kernel.org/r/20251109-qcom-cphy-v1-0-165f7e79b0e1@ixit.cz
---
David Heidelberg (8):
media: qcom: camss: csiphy: Introduce PHY configuration
media: qcom: camss: csiphy-3ph: Use odd bits for configuring C-PHY lanes
media: qcom: camss: Prepare CSID for C-PHY support
media: qcom: camss: Initialize lanes after lane configuration is available
media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 C-PHY init
media: qcom: camss: csiphy-3ph: Update Gen2 v1.1 MIPI CSI-2 C-PHY init
media: qcom: camss: Account for C-PHY when calculating link frequency
media: qcom: camss: Enable C-PHY where available
.../media/platform/qcom/camss/camss-csid-gen2.c | 1 +
drivers/media/platform/qcom/camss/camss-csid.c | 12 +-
drivers/media/platform/qcom/camss/camss-csid.h | 6 +
.../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 243 +++++++++++++++++----
drivers/media/platform/qcom/camss/camss-csiphy.c | 6 +-
drivers/media/platform/qcom/camss/camss-csiphy.h | 2 +
drivers/media/platform/qcom/camss/camss.c | 36 ++-
drivers/media/platform/qcom/camss/camss.h | 2 +-
8 files changed, 245 insertions(+), 63 deletions(-)
---
base-commit: b99ae45861eccff1e1d8c7b05a13650be805d437
change-id: 20251109-qcom-cphy-bb8cbda1c644
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH v7 1/8] media: qcom: camss: csiphy: Introduce PHY configuration
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
Read PHY configuration from the device-tree bus-type and save it into
the csiphy structure for later use.
For C-PHY, skip clock line configuration, as there is none.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss-csiphy.h | 2 ++
drivers/media/platform/qcom/camss/camss.c | 8 ++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 9d9657b82f748..2ebb307be18ba 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -25,21 +25,23 @@
struct csiphy_lane {
u8 pos;
u8 pol;
};
/**
* struct csiphy_lanes_cfg - CSIPHY lanes configuration
+ * @phy_cfg: interface selection (C-PHY or D-PHY)
* @num_data: number of data lanes
* @data: data lanes configuration
* @clk: clock lane configuration (only for D-PHY)
*/
struct csiphy_lanes_cfg {
+ enum v4l2_mbus_type phy_cfg;
int num_data;
struct csiphy_lane *data;
struct csiphy_lane clk;
};
struct csiphy_csi2_cfg {
struct csiphy_lanes_cfg lane_cfg;
};
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 2123f6388e3d7..072c428e25166 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4761,19 +4761,23 @@ static int camss_parse_endpoint_node(struct device *dev,
if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
dev_err(dev, "Unsupported bus type %d\n", vep.bus_type);
return -EINVAL;
}
csd->interface.csiphy_id = vep.base.port;
mipi_csi2 = &vep.bus.mipi_csi2;
- lncfg->clk.pos = mipi_csi2->clock_lane;
- lncfg->clk.pol = mipi_csi2->lane_polarities[0];
lncfg->num_data = mipi_csi2->num_data_lanes;
+ lncfg->phy_cfg = vep.bus_type;
+
+ if (lncfg->phy_cfg != V4L2_MBUS_CSI2_CPHY) {
+ lncfg->clk.pos = mipi_csi2->clock_lane;
+ lncfg->clk.pol = mipi_csi2->lane_polarities[0];
+ }
lncfg->data = devm_kcalloc(dev,
lncfg->num_data, sizeof(*lncfg->data),
GFP_KERNEL);
if (!lncfg->data)
return -ENOMEM;
for (i = 0; i < lncfg->num_data; i++) {
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 1/8] media: qcom: camss: csiphy: Introduce PHY configuration
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
Read PHY configuration from the device-tree bus-type and save it into
the csiphy structure for later use.
For C-PHY, skip clock line configuration, as there is none.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss-csiphy.h | 2 ++
drivers/media/platform/qcom/camss/camss.c | 8 ++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 9d9657b82f748..2ebb307be18ba 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -25,21 +25,23 @@
struct csiphy_lane {
u8 pos;
u8 pol;
};
/**
* struct csiphy_lanes_cfg - CSIPHY lanes configuration
+ * @phy_cfg: interface selection (C-PHY or D-PHY)
* @num_data: number of data lanes
* @data: data lanes configuration
* @clk: clock lane configuration (only for D-PHY)
*/
struct csiphy_lanes_cfg {
+ enum v4l2_mbus_type phy_cfg;
int num_data;
struct csiphy_lane *data;
struct csiphy_lane clk;
};
struct csiphy_csi2_cfg {
struct csiphy_lanes_cfg lane_cfg;
};
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 2123f6388e3d7..072c428e25166 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4761,19 +4761,23 @@ static int camss_parse_endpoint_node(struct device *dev,
if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
dev_err(dev, "Unsupported bus type %d\n", vep.bus_type);
return -EINVAL;
}
csd->interface.csiphy_id = vep.base.port;
mipi_csi2 = &vep.bus.mipi_csi2;
- lncfg->clk.pos = mipi_csi2->clock_lane;
- lncfg->clk.pol = mipi_csi2->lane_polarities[0];
lncfg->num_data = mipi_csi2->num_data_lanes;
+ lncfg->phy_cfg = vep.bus_type;
+
+ if (lncfg->phy_cfg != V4L2_MBUS_CSI2_CPHY) {
+ lncfg->clk.pos = mipi_csi2->clock_lane;
+ lncfg->clk.pol = mipi_csi2->lane_polarities[0];
+ }
lncfg->data = devm_kcalloc(dev,
lncfg->num_data, sizeof(*lncfg->data),
GFP_KERNEL);
if (!lncfg->data)
return -ENOMEM;
for (i = 0; i < lncfg->num_data; i++) {
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v7 2/8] media: qcom: camss: csiphy-3ph: Use odd bits for configuring C-PHY lanes
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
So far, only D-PHY mode was supported, which uses even bits when enabling
or masking lanes. For C-PHY configuration, the hardware instead requires
using the odd bits.
Since there can be unrecognized configuration allow returning failure.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 39 +++++++++++++++++-----
1 file changed, 31 insertions(+), 8 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 dac8d2ecf7995..fa24fc9706748 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
@@ -9,16 +9,17 @@
*/
#include "camss.h"
#include "camss-csiphy.h"
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/media-bus-format.h>
#define CSIPHY_3PH_LNn_CFG1(n) (0x000 + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG1_SWI_REC_DLY_PRG (BIT(7) | BIT(6))
#define CSIPHY_3PH_LNn_CFG2(n) (0x004 + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG2_LP_REC_EN_INT BIT(3)
#define CSIPHY_3PH_LNn_CFG3(n) (0x008 + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG4(n) (0x00c + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS 0xa4
@@ -1108,23 +1109,32 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
writel_relaxed(val, csiphy->base + r->reg_addr);
if (r->delay_us)
udelay(r->delay_us);
}
}
static u8 csiphy_get_lane_mask(struct csiphy_lanes_cfg *lane_cfg)
{
- u8 lane_mask;
- int i;
+ u8 lane_mask = 0;
+ u8 offset = 0;
- lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+ switch (lane_cfg->phy_cfg) {
+ case V4L2_MBUS_CSI2_CPHY:
+ offset = 1;
+ break;
+ case V4L2_MBUS_CSI2_DPHY:
+ lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+ break;
+ default:
+ break;
+ }
- for (i = 0; i < lane_cfg->num_data; i++)
- lane_mask |= 1 << lane_cfg->data[i].pos;
+ for (int i = 0; i < lane_cfg->num_data; i++)
+ lane_mask |= BIT(lane_cfg->data[i].pos + offset);
return lane_mask;
}
static bool csiphy_is_gen2(u32 version)
{
bool ret = false;
@@ -1155,19 +1165,32 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
struct csiphy_device_regs *regs = csiphy->regs;
u8 settle_cnt;
u8 val;
int i;
settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
- val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
- for (i = 0; i < c->num_data; i++)
- val |= BIT(c->data[i].pos * 2);
+ val = 0;
+
+ switch (c->phy_cfg) {
+ case V4L2_MBUS_CSI2_CPHY:
+ for (i = 0; i < c->num_data; i++)
+ val |= BIT((c->data[i].pos * 2) + 1);
+ break;
+ case V4L2_MBUS_CSI2_DPHY:
+ val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+
+ for (i = 0; i < c->num_data; i++)
+ val |= BIT(c->data[i].pos * 2);
+ break;
+ default:
+ WARN_ONCE(1, "Unsupported bus type %d!\n", c->phy_cfg);
+ }
writel_relaxed(val, csiphy->base +
CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 5));
val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
writel_relaxed(val, csiphy->base +
CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 6));
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 2/8] media: qcom: camss: csiphy-3ph: Use odd bits for configuring C-PHY lanes
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
So far, only D-PHY mode was supported, which uses even bits when enabling
or masking lanes. For C-PHY configuration, the hardware instead requires
using the odd bits.
Since there can be unrecognized configuration allow returning failure.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 39 +++++++++++++++++-----
1 file changed, 31 insertions(+), 8 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 dac8d2ecf7995..fa24fc9706748 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
@@ -9,16 +9,17 @@
*/
#include "camss.h"
#include "camss-csiphy.h"
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/media-bus-format.h>
#define CSIPHY_3PH_LNn_CFG1(n) (0x000 + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG1_SWI_REC_DLY_PRG (BIT(7) | BIT(6))
#define CSIPHY_3PH_LNn_CFG2(n) (0x004 + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG2_LP_REC_EN_INT BIT(3)
#define CSIPHY_3PH_LNn_CFG3(n) (0x008 + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG4(n) (0x00c + 0x100 * (n))
#define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS 0xa4
@@ -1108,23 +1109,32 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
writel_relaxed(val, csiphy->base + r->reg_addr);
if (r->delay_us)
udelay(r->delay_us);
}
}
static u8 csiphy_get_lane_mask(struct csiphy_lanes_cfg *lane_cfg)
{
- u8 lane_mask;
- int i;
+ u8 lane_mask = 0;
+ u8 offset = 0;
- lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+ switch (lane_cfg->phy_cfg) {
+ case V4L2_MBUS_CSI2_CPHY:
+ offset = 1;
+ break;
+ case V4L2_MBUS_CSI2_DPHY:
+ lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+ break;
+ default:
+ break;
+ }
- for (i = 0; i < lane_cfg->num_data; i++)
- lane_mask |= 1 << lane_cfg->data[i].pos;
+ for (int i = 0; i < lane_cfg->num_data; i++)
+ lane_mask |= BIT(lane_cfg->data[i].pos + offset);
return lane_mask;
}
static bool csiphy_is_gen2(u32 version)
{
bool ret = false;
@@ -1155,19 +1165,32 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
struct csiphy_device_regs *regs = csiphy->regs;
u8 settle_cnt;
u8 val;
int i;
settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
- val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
- for (i = 0; i < c->num_data; i++)
- val |= BIT(c->data[i].pos * 2);
+ val = 0;
+
+ switch (c->phy_cfg) {
+ case V4L2_MBUS_CSI2_CPHY:
+ for (i = 0; i < c->num_data; i++)
+ val |= BIT((c->data[i].pos * 2) + 1);
+ break;
+ case V4L2_MBUS_CSI2_DPHY:
+ val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
+
+ for (i = 0; i < c->num_data; i++)
+ val |= BIT(c->data[i].pos * 2);
+ break;
+ default:
+ WARN_ONCE(1, "Unsupported bus type %d!\n", c->phy_cfg);
+ }
writel_relaxed(val, csiphy->base +
CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 5));
val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
writel_relaxed(val, csiphy->base +
CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 6));
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 2/8] media: qcom: camss: csiphy-3ph: Use odd bits for configuring C-PHY lanes
2026-06-05 13:14 ` David Heidelberg
(?)
@ 2026-06-05 13:30 ` sashiko-bot
-1 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-05 13:30 UTC (permalink / raw)
To: David Heidelberg via B4 Relay; +Cc: linux-media
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Commit message claims to return failure but code does not implement error propagation.
--
commit da5484f83dd0e6203bd1f47a4299fe8d0bb8c433
Author: David Heidelberg <david@ixit.cz>
media: qcom: camss: csiphy-3ph: Use odd bits for configuring C-PHY lanes
This commit adds support for C-PHY lane configuration by utilizing odd
bits instead of the even bits used for D-PHY mode. It updates the lane
mask calculation and enablement routines for C-PHY hardware requirements.
> Since there can be unrecognized configuration allow returning failure.
[Severity: Medium]
The commit message explicitly states "allow returning failure" for
unrecognized configurations, but does the code actually implement this?
Looking at csiphy_get_lane_mask() and csiphy_lanes_enable(), the code does
not appear to propagate any errors. Could the commit message be updated
to reflect the use of warnings, or should the functions be changed to return
an error code?
> 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 dac8d2ecf7995..fa24fc9706748 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
[ ... ]
> @@ -1113,13 +1114,22 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
>
> static u8 csiphy_get_lane_mask(struct csiphy_lanes_cfg *lane_cfg)
> {
> - u8 lane_mask;
> - int i;
> + u8 lane_mask = 0;
> + u8 offset = 0;
>
> - lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> + switch (lane_cfg->phy_cfg) {
> + case V4L2_MBUS_CSI2_CPHY:
> + offset = 1;
> + break;
> + case V4L2_MBUS_CSI2_DPHY:
> + lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> + break;
> + default:
> + break;
> + }
[Severity: Medium]
In csiphy_get_lane_mask(), the default case silently falls through and
returns 0. Is this intended to be treated as a failure condition?
[ ... ]
> @@ -1160,9 +1170,22 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
[ ... ]
> - val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> - for (i = 0; i < c->num_data; i++)
> - val |= BIT(c->data[i].pos * 2);
> + val = 0;
> +
> + switch (c->phy_cfg) {
> + case V4L2_MBUS_CSI2_CPHY:
> + for (i = 0; i < c->num_data; i++)
> + val |= BIT((c->data[i].pos * 2) + 1);
> + break;
> + case V4L2_MBUS_CSI2_DPHY:
> + val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> +
> + for (i = 0; i < c->num_data; i++)
> + val |= BIT(c->data[i].pos * 2);
> + break;
> + default:
> + WARN_ONCE(1, "Unsupported bus type %d!\n", c->phy_cfg);
> + }
[Severity: Medium]
Similarly, since csiphy_lanes_enable() has a void return type, the caller
has no way to detect this failure. While misconfiguration might be
prevented earlier during device tree parsing, does this function need to be
updated to actually return the failure mentioned in the commit message?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-qcom-cphy-v7-0-426c37e9008f@ixit.cz?part=2
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 2/8] media: qcom: camss: csiphy-3ph: Use odd bits for configuring C-PHY lanes
2026-06-05 13:14 ` David Heidelberg
(?)
(?)
@ 2026-06-08 18:16 ` Frank Li
2026-06-08 19:32 ` David Heidelberg
-1 siblings, 1 reply; 28+ messages in thread
From: Frank Li @ 2026-06-08 18:16 UTC (permalink / raw)
To: David Heidelberg
Cc: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain,
Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel
On Fri, Jun 05, 2026 at 03:14:40PM +0200, David Heidelberg wrote:
> So far, only D-PHY mode was supported, which uses even bits when enabling
> or masking lanes. For C-PHY configuration, the hardware instead requires
> using the odd bits.
>
> Since there can be unrecognized configuration allow returning failure.
>
> Acked-by: Cory Keitz <ckeitz@amazon.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
> .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 39 +++++++++++++++++-----
> 1 file changed, 31 insertions(+), 8 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 dac8d2ecf7995..fa24fc9706748 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
> @@ -9,16 +9,17 @@
> */
>
> #include "camss.h"
> #include "camss-csiphy.h"
>
> #include <linux/delay.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> +#include <linux/media-bus-format.h>
>
> #define CSIPHY_3PH_LNn_CFG1(n) (0x000 + 0x100 * (n))
> #define CSIPHY_3PH_LNn_CFG1_SWI_REC_DLY_PRG (BIT(7) | BIT(6))
> #define CSIPHY_3PH_LNn_CFG2(n) (0x004 + 0x100 * (n))
> #define CSIPHY_3PH_LNn_CFG2_LP_REC_EN_INT BIT(3)
> #define CSIPHY_3PH_LNn_CFG3(n) (0x008 + 0x100 * (n))
> #define CSIPHY_3PH_LNn_CFG4(n) (0x00c + 0x100 * (n))
> #define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS 0xa4
> @@ -1108,23 +1109,32 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
> writel_relaxed(val, csiphy->base + r->reg_addr);
> if (r->delay_us)
> udelay(r->delay_us);
> }
> }
>
> static u8 csiphy_get_lane_mask(struct csiphy_lanes_cfg *lane_cfg)
> {
> - u8 lane_mask;
> - int i;
> + u8 lane_mask = 0;
> + u8 offset = 0;
>
> - lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> + switch (lane_cfg->phy_cfg) {
> + case V4L2_MBUS_CSI2_CPHY:
> + offset = 1;
> + break;
> + case V4L2_MBUS_CSI2_DPHY:
> + lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> + break;
> + default:
> + break;
> + }
>
> - for (i = 0; i < lane_cfg->num_data; i++)
> - lane_mask |= 1 << lane_cfg->data[i].pos;
> + for (int i = 0; i < lane_cfg->num_data; i++)
> + lane_mask |= BIT(lane_cfg->data[i].pos + offset);
>
> return lane_mask;
> }
>
> static bool csiphy_is_gen2(u32 version)
> {
> bool ret = false;
>
> @@ -1155,19 +1165,32 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
> struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
> struct csiphy_device_regs *regs = csiphy->regs;
> u8 settle_cnt;
> u8 val;
> int i;
>
> settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
>
> - val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> - for (i = 0; i < c->num_data; i++)
> - val |= BIT(c->data[i].pos * 2);
> + val = 0;
> +
> + switch (c->phy_cfg) {
> + case V4L2_MBUS_CSI2_CPHY:
> + for (i = 0; i < c->num_data; i++)
> + val |= BIT((c->data[i].pos * 2) + 1);
> + break;
> + case V4L2_MBUS_CSI2_DPHY:
> + val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> +
> + for (i = 0; i < c->num_data; i++)
> + val |= BIT(c->data[i].pos * 2);
> + break;
> + default:
> + WARN_ONCE(1, "Unsupported bus type %d!\n", c->phy_cfg);
> + }
can you use similar method as csiphy_get_lane_mask()
Frank
>
> writel_relaxed(val, csiphy->base +
> CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 5));
>
> val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
> writel_relaxed(val, csiphy->base +
> CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 6));
>
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 2/8] media: qcom: camss: csiphy-3ph: Use odd bits for configuring C-PHY lanes
2026-06-08 18:16 ` Frank Li
@ 2026-06-08 19:32 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-08 19:32 UTC (permalink / raw)
To: Frank Li
Cc: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain,
Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel
On 08/06/2026 20:16, Frank Li wrote:
> On Fri, Jun 05, 2026 at 03:14:40PM +0200, David Heidelberg wrote:
>> So far, only D-PHY mode was supported, which uses even bits when enabling
>> or masking lanes. For C-PHY configuration, the hardware instead requires
>> using the odd bits.
>>
>> Since there can be unrecognized configuration allow returning failure.
>>
>> Acked-by: Cory Keitz <ckeitz@amazon.com>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
>> .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 39 +++++++++++++++++-----
>> 1 file changed, 31 insertions(+), 8 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 dac8d2ecf7995..fa24fc9706748 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
>> @@ -9,16 +9,17 @@
>> */
>>
>> #include "camss.h"
>> #include "camss-csiphy.h"
>>
>> #include <linux/delay.h>
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> +#include <linux/media-bus-format.h>
>>
>> #define CSIPHY_3PH_LNn_CFG1(n) (0x000 + 0x100 * (n))
>> #define CSIPHY_3PH_LNn_CFG1_SWI_REC_DLY_PRG (BIT(7) | BIT(6))
>> #define CSIPHY_3PH_LNn_CFG2(n) (0x004 + 0x100 * (n))
>> #define CSIPHY_3PH_LNn_CFG2_LP_REC_EN_INT BIT(3)
>> #define CSIPHY_3PH_LNn_CFG3(n) (0x008 + 0x100 * (n))
>> #define CSIPHY_3PH_LNn_CFG4(n) (0x00c + 0x100 * (n))
>> #define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS 0xa4
>> @@ -1108,23 +1109,32 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
>> writel_relaxed(val, csiphy->base + r->reg_addr);
>> if (r->delay_us)
>> udelay(r->delay_us);
>> }
>> }
>>
>> static u8 csiphy_get_lane_mask(struct csiphy_lanes_cfg *lane_cfg)
>> {
>> - u8 lane_mask;
>> - int i;
>> + u8 lane_mask = 0;
>> + u8 offset = 0;
>>
>> - lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
>> + switch (lane_cfg->phy_cfg) {
>> + case V4L2_MBUS_CSI2_CPHY:
>> + offset = 1;
>> + break;
>> + case V4L2_MBUS_CSI2_DPHY:
>> + lane_mask = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
>> + break;
>> + default:
>> + break;
>> + }
>>
>> - for (i = 0; i < lane_cfg->num_data; i++)
>> - lane_mask |= 1 << lane_cfg->data[i].pos;
>> + for (int i = 0; i < lane_cfg->num_data; i++)
>> + lane_mask |= BIT(lane_cfg->data[i].pos + offset);
>>
>> return lane_mask;
>> }
>>
>> static bool csiphy_is_gen2(u32 version)
>> {
>> bool ret = false;
>>
>> @@ -1155,19 +1165,32 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>> struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
>> struct csiphy_device_regs *regs = csiphy->regs;
>> u8 settle_cnt;
>> u8 val;
>> int i;
>>
>> settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
>>
>> - val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
>> - for (i = 0; i < c->num_data; i++)
>> - val |= BIT(c->data[i].pos * 2);
>> + val = 0;
>> +
>> + switch (c->phy_cfg) {
>> + case V4L2_MBUS_CSI2_CPHY:
>> + for (i = 0; i < c->num_data; i++)
>> + val |= BIT((c->data[i].pos * 2) + 1);
>> + break;
>> + case V4L2_MBUS_CSI2_DPHY:
>> + val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
>> +
>> + for (i = 0; i < c->num_data; i++)
>> + val |= BIT(c->data[i].pos * 2);
>> + break;
>> + default:
>> + WARN_ONCE(1, "Unsupported bus type %d!\n", c->phy_cfg);
>> + }
>
> can you use similar method as csiphy_get_lane_mask()
>
Done, look better, but I guess we seriously need the refactor/split done by
Bryan as I looking at the current code.
David
> Frank
>>
>> writel_relaxed(val, csiphy->base +
>> CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 5));
>>
>> val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
>> writel_relaxed(val, csiphy->base +
>> CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 6));
>>
>>
>> --
>> 2.53.0
>>
--
David Heidelberg
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 3/8] media: qcom: camss: Prepare CSID for C-PHY support
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
Inherit C-PHY information from CSIPHY, so we can configure CSID
properly.
CSI2_RX_CFG0_PHY_TYPE_SEL must be set to 1, when C-PHY mode is used.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Cory Keitz <ckeitz@amazon.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss-csid-gen2.c | 1 +
drivers/media/platform/qcom/camss/camss-csid.c | 5 +++++
drivers/media/platform/qcom/camss/camss-csid.h | 6 ++++++
3 files changed, 12 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index eadcb2f7e3aaa..a5b406cc8ead3 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -178,16 +178,17 @@ static void __csid_configure_rx(struct csid_device *csid,
int val;
if (!lane_cnt)
lane_cnt = 4;
val = (lane_cnt - 1) << CSI2_RX_CFG0_NUM_ACTIVE_LANES;
val |= phy->lane_assign << CSI2_RX_CFG0_DL0_INPUT_SEL;
val |= phy->csiphy_id << CSI2_RX_CFG0_PHY_NUM_SEL;
+ val |= csid->phy.phy_sel << CSI2_RX_CFG0_PHY_TYPE_SEL;
writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
if (vc > 3)
val |= 1 << CSI2_RX_CFG1_VC_MODE;
val |= 1 << CSI2_RX_CFG1_MISR_EN;
writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
}
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 48459b46a981b..bcc34ac9dd212 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -1286,16 +1286,21 @@ static int csid_link_setup(struct media_entity *entity,
/* do no allow a link from CSIPHY to CSID */
if (!csiphy->cfg.csi2)
return -EPERM;
csid->phy.csiphy_id = csiphy->id;
lane_cfg = &csiphy->cfg.csi2->lane_cfg;
csid->phy.lane_cnt = lane_cfg->num_data;
+ if (lane_cfg->phy_cfg == V4L2_MBUS_CSI2_CPHY)
+ csid->phy.phy_sel = CSID_PHY_SEL_CPHY;
+ else
+ csid->phy.phy_sel = CSID_PHY_SEL_DPHY;
+
csid->phy.lane_assign = csid_get_lane_assign(lane_cfg, lane_cfg->num_data);
csid->tpg_linked = false;
}
}
/* Decide which virtual channels to enable based on which source pads are enabled */
if (local->flags & MEDIA_PAD_FL_SOURCE) {
struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
struct csid_device *csid = v4l2_get_subdevdata(sd);
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 5296b10f6bac8..e65590b0df69f 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -39,16 +39,21 @@ enum csid_testgen_mode {
CSID_PAYLOAD_MODE_USER_SPECIFIED = 6,
CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN1 = 6, /* excluding disabled */
CSID_PAYLOAD_MODE_COMPLEX_PATTERN = 7,
CSID_PAYLOAD_MODE_COLOR_BOX = 8,
CSID_PAYLOAD_MODE_COLOR_BARS = 9,
CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2 = 9, /* excluding disabled */
};
+enum csid_phy_sel {
+ CSID_PHY_SEL_DPHY = 0,
+ CSID_PHY_SEL_CPHY = 1
+};
+
struct csid_format_info {
u32 code;
u8 data_type;
u8 decode_format;
u8 bpp;
u8 spp; /* bus samples per pixel */
};
@@ -65,16 +70,17 @@ struct csid_testgen_config {
};
struct csid_phy_config {
u8 csiphy_id;
u8 lane_cnt;
u32 lane_assign;
u32 en_vc;
u8 need_vc_update;
+ enum csid_phy_sel phy_sel;
};
struct csid_device;
struct csid_hw_ops {
/*
* configure_stream - Configures and starts CSID input stream
* @csid: CSID device
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 3/8] media: qcom: camss: Prepare CSID for C-PHY support
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
Inherit C-PHY information from CSIPHY, so we can configure CSID
properly.
CSI2_RX_CFG0_PHY_TYPE_SEL must be set to 1, when C-PHY mode is used.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Cory Keitz <ckeitz@amazon.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss-csid-gen2.c | 1 +
drivers/media/platform/qcom/camss/camss-csid.c | 5 +++++
drivers/media/platform/qcom/camss/camss-csid.h | 6 ++++++
3 files changed, 12 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
index eadcb2f7e3aaa..a5b406cc8ead3 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
@@ -178,16 +178,17 @@ static void __csid_configure_rx(struct csid_device *csid,
int val;
if (!lane_cnt)
lane_cnt = 4;
val = (lane_cnt - 1) << CSI2_RX_CFG0_NUM_ACTIVE_LANES;
val |= phy->lane_assign << CSI2_RX_CFG0_DL0_INPUT_SEL;
val |= phy->csiphy_id << CSI2_RX_CFG0_PHY_NUM_SEL;
+ val |= csid->phy.phy_sel << CSI2_RX_CFG0_PHY_TYPE_SEL;
writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
if (vc > 3)
val |= 1 << CSI2_RX_CFG1_VC_MODE;
val |= 1 << CSI2_RX_CFG1_MISR_EN;
writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
}
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 48459b46a981b..bcc34ac9dd212 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -1286,16 +1286,21 @@ static int csid_link_setup(struct media_entity *entity,
/* do no allow a link from CSIPHY to CSID */
if (!csiphy->cfg.csi2)
return -EPERM;
csid->phy.csiphy_id = csiphy->id;
lane_cfg = &csiphy->cfg.csi2->lane_cfg;
csid->phy.lane_cnt = lane_cfg->num_data;
+ if (lane_cfg->phy_cfg == V4L2_MBUS_CSI2_CPHY)
+ csid->phy.phy_sel = CSID_PHY_SEL_CPHY;
+ else
+ csid->phy.phy_sel = CSID_PHY_SEL_DPHY;
+
csid->phy.lane_assign = csid_get_lane_assign(lane_cfg, lane_cfg->num_data);
csid->tpg_linked = false;
}
}
/* Decide which virtual channels to enable based on which source pads are enabled */
if (local->flags & MEDIA_PAD_FL_SOURCE) {
struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
struct csid_device *csid = v4l2_get_subdevdata(sd);
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 5296b10f6bac8..e65590b0df69f 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -39,16 +39,21 @@ enum csid_testgen_mode {
CSID_PAYLOAD_MODE_USER_SPECIFIED = 6,
CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN1 = 6, /* excluding disabled */
CSID_PAYLOAD_MODE_COMPLEX_PATTERN = 7,
CSID_PAYLOAD_MODE_COLOR_BOX = 8,
CSID_PAYLOAD_MODE_COLOR_BARS = 9,
CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2 = 9, /* excluding disabled */
};
+enum csid_phy_sel {
+ CSID_PHY_SEL_DPHY = 0,
+ CSID_PHY_SEL_CPHY = 1
+};
+
struct csid_format_info {
u32 code;
u8 data_type;
u8 decode_format;
u8 bpp;
u8 spp; /* bus samples per pixel */
};
@@ -65,16 +70,17 @@ struct csid_testgen_config {
};
struct csid_phy_config {
u8 csiphy_id;
u8 lane_cnt;
u32 lane_assign;
u32 en_vc;
u8 need_vc_update;
+ enum csid_phy_sel phy_sel;
};
struct csid_device;
struct csid_hw_ops {
/*
* configure_stream - Configures and starts CSID input stream
* @csid: CSID device
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 3/8] media: qcom: camss: Prepare CSID for C-PHY support
2026-06-05 13:14 ` David Heidelberg
(?)
@ 2026-06-05 13:35 ` sashiko-bot
-1 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-05 13:35 UTC (permalink / raw)
To: David Heidelberg via B4 Relay; +Cc: linux-media
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] CSID PHY selection state (`phy_sel`) is not reset when switching the link to the Test Pattern Generator (TPG), potentially leaving the hardware in C-PHY mode for TPG data.
--
commit 79ab3be6b44125cac8b0fb89e7dbad51a804a8bf
Author: David Heidelberg <david@ixit.cz>
media: qcom: camss: Prepare CSID for C-PHY support
This commit updates the CSID configuration to inherit C-PHY information
from CSIPHY, ensuring the CSI2_RX_CFG0_PHY_TYPE_SEL register is properly
set when C-PHY mode is used.
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 48459b46a981b..bcc34ac9dd212 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
[ ... ]
> @@ -1291,6 +1291,11 @@ static int csid_link_setup(struct media_entity *entity,
>
> lane_cfg = &csiphy->cfg.csi2->lane_cfg;
> csid->phy.lane_cnt = lane_cfg->num_data;
> + if (lane_cfg->phy_cfg == V4L2_MBUS_CSI2_CPHY)
> + csid->phy.phy_sel = CSID_PHY_SEL_CPHY;
> + else
> + csid->phy.phy_sel = CSID_PHY_SEL_DPHY;
> +
[Severity: Medium]
Does this leave the phy_sel state uninitialized when the link is switched
to the Test Pattern Generator (TPG)?
In csid_link_setup(), when switching the link to the internal TPG, the newly
added phy_sel field is not updated:
csid_link_setup() {
...
if (sd->grp_id == TPG_GRP_ID) {
tpg = v4l2_get_subdevdata(sd);
csid->phy.lane_cnt = tpg->res->lane_cnt;
csid->phy.csiphy_id = tpg->id;
csid->phy.lane_assign = csid_get_lane_assign(NULL, csid->phy.lane_cnt);
csid->tpg_linked = true;
/* csid->phy.phy_sel is not reset here */
} else {
...
If a C-PHY sensor was previously linked, phy_sel will retain the stale value
CSID_PHY_SEL_CPHY.
When streaming starts, __csid_configure_rx() writes this stale phy_sel value
into the CSI2_RX_CFG0_PHY_TYPE_SEL register, even for TPG streams.
Because the TPG bypasses the C-PHY 3-phase encoding, applying C-PHY decoding
rules to TPG data might cause the hardware to misinterpret the stream and
break the test pattern generator.
> csid->phy.lane_assign = csid_get_lane_assign(lane_cfg, lane_cfg->num_data);
> csid->tpg_linked = false;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-qcom-cphy-v7-0-426c37e9008f@ixit.cz?part=3
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 3/8] media: qcom: camss: Prepare CSID for C-PHY support
2026-06-05 13:14 ` David Heidelberg
(?)
(?)
@ 2026-06-08 18:18 ` Frank Li
-1 siblings, 0 replies; 28+ messages in thread
From: Frank Li @ 2026-06-08 18:18 UTC (permalink / raw)
To: David Heidelberg
Cc: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain,
Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel
On Fri, Jun 05, 2026 at 03:14:41PM +0200, David Heidelberg wrote:
> Inherit C-PHY information from CSIPHY, so we can configure CSID
> properly.
>
> CSI2_RX_CFG0_PHY_TYPE_SEL must be set to 1, when C-PHY mode is used.
>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Acked-by: Cory Keitz <ckeitz@amazon.com>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
> drivers/media/platform/qcom/camss/camss-csid-gen2.c | 1 +
> drivers/media/platform/qcom/camss/camss-csid.c | 5 +++++
> drivers/media/platform/qcom/camss/camss-csid.h | 6 ++++++
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> index eadcb2f7e3aaa..a5b406cc8ead3 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> @@ -178,16 +178,17 @@ static void __csid_configure_rx(struct csid_device *csid,
> int val;
>
> if (!lane_cnt)
> lane_cnt = 4;
>
> val = (lane_cnt - 1) << CSI2_RX_CFG0_NUM_ACTIVE_LANES;
> val |= phy->lane_assign << CSI2_RX_CFG0_DL0_INPUT_SEL;
> val |= phy->csiphy_id << CSI2_RX_CFG0_PHY_NUM_SEL;
> + val |= csid->phy.phy_sel << CSI2_RX_CFG0_PHY_TYPE_SEL;
It is fine for now. Suggest change to FIELD_PREP() in future.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
>
> val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
> if (vc > 3)
> val |= 1 << CSI2_RX_CFG1_VC_MODE;
> val |= 1 << CSI2_RX_CFG1_MISR_EN;
> writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
> }
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 48459b46a981b..bcc34ac9dd212 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -1286,16 +1286,21 @@ static int csid_link_setup(struct media_entity *entity,
> /* do no allow a link from CSIPHY to CSID */
> if (!csiphy->cfg.csi2)
> return -EPERM;
>
> csid->phy.csiphy_id = csiphy->id;
>
> lane_cfg = &csiphy->cfg.csi2->lane_cfg;
> csid->phy.lane_cnt = lane_cfg->num_data;
> + if (lane_cfg->phy_cfg == V4L2_MBUS_CSI2_CPHY)
> + csid->phy.phy_sel = CSID_PHY_SEL_CPHY;
> + else
> + csid->phy.phy_sel = CSID_PHY_SEL_DPHY;
> +
> csid->phy.lane_assign = csid_get_lane_assign(lane_cfg, lane_cfg->num_data);
> csid->tpg_linked = false;
> }
> }
> /* Decide which virtual channels to enable based on which source pads are enabled */
> if (local->flags & MEDIA_PAD_FL_SOURCE) {
> struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> struct csid_device *csid = v4l2_get_subdevdata(sd);
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
> index 5296b10f6bac8..e65590b0df69f 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.h
> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
> @@ -39,16 +39,21 @@ enum csid_testgen_mode {
> CSID_PAYLOAD_MODE_USER_SPECIFIED = 6,
> CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN1 = 6, /* excluding disabled */
> CSID_PAYLOAD_MODE_COMPLEX_PATTERN = 7,
> CSID_PAYLOAD_MODE_COLOR_BOX = 8,
> CSID_PAYLOAD_MODE_COLOR_BARS = 9,
> CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2 = 9, /* excluding disabled */
> };
>
> +enum csid_phy_sel {
> + CSID_PHY_SEL_DPHY = 0,
> + CSID_PHY_SEL_CPHY = 1
> +};
> +
> struct csid_format_info {
> u32 code;
> u8 data_type;
> u8 decode_format;
> u8 bpp;
> u8 spp; /* bus samples per pixel */
> };
>
> @@ -65,16 +70,17 @@ struct csid_testgen_config {
> };
>
> struct csid_phy_config {
> u8 csiphy_id;
> u8 lane_cnt;
> u32 lane_assign;
> u32 en_vc;
> u8 need_vc_update;
> + enum csid_phy_sel phy_sel;
> };
>
> struct csid_device;
>
> struct csid_hw_ops {
> /*
> * configure_stream - Configures and starts CSID input stream
> * @csid: CSID device
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 4/8] media: qcom: camss: Initialize lanes after lane configuration is available
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
The lanes must not be initialized before the driver has access to
the lane configuration, as it depends on whether D-PHY or C-PHY mode
is in use. Move the lane initialization to csiphy_lanes_enable which is
called when the configuration structures are available.
Co-developed-by: Petr Hodina <phodina@protonmail.com>
Signed-off-by: Petr Hodina <phodina@protonmail.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 131 +++++++++++++++------
1 file changed, 93 insertions(+), 38 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 fa24fc9706748..c76b9c352ea02 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
@@ -1163,16 +1163,108 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
s64 link_freq, u8 lane_mask)
{
struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
struct csiphy_device_regs *regs = csiphy->regs;
u8 settle_cnt;
u8 val;
int i;
+ switch (csiphy->camss->res->version) {
+ case CAMSS_845:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sdm845[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
+ }
+ break;
+ case CAMSS_2290:
+ case CAMSS_6150:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_qcm2290[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290);
+ }
+ break;
+ case CAMSS_6350:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm6350[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm6350);
+ }
+ break;
+ case CAMSS_7280:
+ case CAMSS_8250:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm8250[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
+ }
+ break;
+ case CAMSS_8280XP:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sc8280xp[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
+ }
+ break;
+ case CAMSS_X1E80100:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_x1e80100[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_x1e80100);
+ }
+ break;
+ case CAMSS_8550:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm8550[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550);
+ }
+ break;
+ case CAMSS_8650:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm8650[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650);
+ }
+ break;
+ case CAMSS_8300:
+ case CAMSS_8775P:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sa8775p[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
+ }
+ break;
+ default:
+ break;
+ }
+
+ if (!regs->lane_regs && c->phy_cfg == V4L2_MBUS_CSI2_CPHY)
+ WARN_ONCE(1, "Missing lane_regs definition for C-PHY!\n");
+
settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
val = 0;
switch (c->phy_cfg) {
case V4L2_MBUS_CSI2_CPHY:
for (i = 0; i < c->num_data; i++)
val |= BIT((c->data[i].pos * 2) + 1);
@@ -1231,63 +1323,26 @@ static int csiphy_init(struct csiphy_device *csiphy)
struct device *dev = csiphy->camss->dev;
struct csiphy_device_regs *regs;
regs = devm_kmalloc(dev, sizeof(*regs), GFP_KERNEL);
if (!regs)
return -ENOMEM;
csiphy->regs = regs;
- regs->offset = 0x800;
regs->common_status_offset = 0xb0;
switch (csiphy->camss->res->version) {
- case CAMSS_845:
- regs->lane_regs = &lane_regs_sdm845[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
- break;
- case CAMSS_2290:
- case CAMSS_6150:
- regs->lane_regs = &lane_regs_qcm2290[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290);
- break;
- case CAMSS_6350:
- regs->lane_regs = &lane_regs_sm6350[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm6350);
- break;
- case CAMSS_7280:
- case CAMSS_8250:
- regs->lane_regs = &lane_regs_sm8250[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
- break;
- case CAMSS_8280XP:
- regs->lane_regs = &lane_regs_sc8280xp[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
- break;
case CAMSS_X1E80100:
- regs->lane_regs = &lane_regs_x1e80100[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_x1e80100);
- regs->offset = 0x1000;
- break;
case CAMSS_8550:
- regs->lane_regs = &lane_regs_sm8550[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550);
- regs->offset = 0x1000;
- break;
case CAMSS_8650:
- regs->lane_regs = &lane_regs_sm8650[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650);
regs->offset = 0x1000;
break;
- case CAMSS_8300:
- case CAMSS_8775P:
- regs->lane_regs = &lane_regs_sa8775p[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
- break;
default:
+ regs->offset = 0x800;
break;
}
return 0;
}
const struct csiphy_hw_ops csiphy_ops_3ph_1_0 = {
.get_lane_mask = csiphy_get_lane_mask,
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 4/8] media: qcom: camss: Initialize lanes after lane configuration is available
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
The lanes must not be initialized before the driver has access to
the lane configuration, as it depends on whether D-PHY or C-PHY mode
is in use. Move the lane initialization to csiphy_lanes_enable which is
called when the configuration structures are available.
Co-developed-by: Petr Hodina <phodina@protonmail.com>
Signed-off-by: Petr Hodina <phodina@protonmail.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 131 +++++++++++++++------
1 file changed, 93 insertions(+), 38 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 fa24fc9706748..c76b9c352ea02 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
@@ -1163,16 +1163,108 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
s64 link_freq, u8 lane_mask)
{
struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
struct csiphy_device_regs *regs = csiphy->regs;
u8 settle_cnt;
u8 val;
int i;
+ switch (csiphy->camss->res->version) {
+ case CAMSS_845:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sdm845[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
+ }
+ break;
+ case CAMSS_2290:
+ case CAMSS_6150:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_qcm2290[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290);
+ }
+ break;
+ case CAMSS_6350:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm6350[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm6350);
+ }
+ break;
+ case CAMSS_7280:
+ case CAMSS_8250:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm8250[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
+ }
+ break;
+ case CAMSS_8280XP:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sc8280xp[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
+ }
+ break;
+ case CAMSS_X1E80100:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_x1e80100[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_x1e80100);
+ }
+ break;
+ case CAMSS_8550:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm8550[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550);
+ }
+ break;
+ case CAMSS_8650:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sm8650[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650);
+ }
+ break;
+ case CAMSS_8300:
+ case CAMSS_8775P:
+ if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+ regs->lane_regs = NULL;
+ regs->lane_array_size = 0;
+ } else {
+ regs->lane_regs = &lane_regs_sa8775p[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
+ }
+ break;
+ default:
+ break;
+ }
+
+ if (!regs->lane_regs && c->phy_cfg == V4L2_MBUS_CSI2_CPHY)
+ WARN_ONCE(1, "Missing lane_regs definition for C-PHY!\n");
+
settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate);
val = 0;
switch (c->phy_cfg) {
case V4L2_MBUS_CSI2_CPHY:
for (i = 0; i < c->num_data; i++)
val |= BIT((c->data[i].pos * 2) + 1);
@@ -1231,63 +1323,26 @@ static int csiphy_init(struct csiphy_device *csiphy)
struct device *dev = csiphy->camss->dev;
struct csiphy_device_regs *regs;
regs = devm_kmalloc(dev, sizeof(*regs), GFP_KERNEL);
if (!regs)
return -ENOMEM;
csiphy->regs = regs;
- regs->offset = 0x800;
regs->common_status_offset = 0xb0;
switch (csiphy->camss->res->version) {
- case CAMSS_845:
- regs->lane_regs = &lane_regs_sdm845[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
- break;
- case CAMSS_2290:
- case CAMSS_6150:
- regs->lane_regs = &lane_regs_qcm2290[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290);
- break;
- case CAMSS_6350:
- regs->lane_regs = &lane_regs_sm6350[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm6350);
- break;
- case CAMSS_7280:
- case CAMSS_8250:
- regs->lane_regs = &lane_regs_sm8250[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
- break;
- case CAMSS_8280XP:
- regs->lane_regs = &lane_regs_sc8280xp[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
- break;
case CAMSS_X1E80100:
- regs->lane_regs = &lane_regs_x1e80100[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_x1e80100);
- regs->offset = 0x1000;
- break;
case CAMSS_8550:
- regs->lane_regs = &lane_regs_sm8550[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8550);
- regs->offset = 0x1000;
- break;
case CAMSS_8650:
- regs->lane_regs = &lane_regs_sm8650[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8650);
regs->offset = 0x1000;
break;
- case CAMSS_8300:
- case CAMSS_8775P:
- regs->lane_regs = &lane_regs_sa8775p[0];
- regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
- break;
default:
+ regs->offset = 0x800;
break;
}
return 0;
}
const struct csiphy_hw_ops csiphy_ops_3ph_1_0 = {
.get_lane_mask = csiphy_get_lane_mask,
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 4/8] media: qcom: camss: Initialize lanes after lane configuration is available
2026-06-05 13:14 ` David Heidelberg
(?)
@ 2026-06-05 13:40 ` sashiko-bot
-1 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-05 13:40 UTC (permalink / raw)
To: David Heidelberg via B4 Relay; +Cc: linux-media
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized memory read in csiphy_lanes_enable() for Gen 1 SoCs
--
commit 29ea95d7cd0cd620b740a193084b2b2f90d4327e
Author: David Heidelberg <david@ixit.cz>
media: qcom: camss: Initialize lanes after lane configuration is available
The lanes must not be initialized before the driver has access to
the lane configuration, as it depends on whether D-PHY or C-PHY mode
is in use. Move the lane initialization to csiphy_lanes_enable which is
called when the configuration structures are available.
> 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 fa24fc9706748..c76b9c352ea02 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
> @@ -1168,6 +1168,98 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
> u8 val;
> int i;
>
> + switch (csiphy->camss->res->version) {
[ ... ]
> + case CAMSS_8300:
> + case CAMSS_8775P:
> + if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
> + regs->lane_regs = NULL;
> + regs->lane_array_size = 0;
> + } else {
> + regs->lane_regs = &lane_regs_sa8775p[0];
> + regs->lane_array_size = ARRAY_SIZE(lane_regs_sa8775p);
> + }
> + break;
> + default:
> + break;
> + }
> +
> + if (!regs->lane_regs && c->phy_cfg == V4L2_MBUS_CSI2_CPHY)
[Severity: Medium]
Since regs is allocated with devm_kmalloc() in csiphy_init() and is not
zero-initialized, could regs->lane_regs contain garbage for Gen 1 platforms
that fall through to the default case above?
Because && evaluates left-to-right, this condition evaluates !regs->lane_regs
first, resulting in a read of uninitialized heap memory.
> + WARN_ONCE(1, "Missing lane_regs definition for C-PHY!\n");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-qcom-cphy-v7-0-426c37e9008f@ixit.cz?part=4
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 C-PHY init
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
Add a PHY configuration sequence for the sdm845 which uses a Qualcomm
Gen 2 version 1.1 CSI-2 PHY.
The PHY can be configured as two phase or three phase in C-PHY or D-PHY
mode. This configuration supports three-phase C-PHY mode.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 78 +++++++++++++++++++++-
1 file changed, 76 insertions(+), 2 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 c76b9c352ea02..d9136e422f45f 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
@@ -45,16 +45,23 @@
#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n) ((offset) + 0x4 * (n))
#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, common_status_offset, n) \
((offset) + (common_status_offset) + 0x4 * (n))
+#define CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(n) \
+ (0x0100 + ((n) * 0x4))
+#define CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(n) \
+ (0x0300 + ((n) * 0x4))
+#define CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(n) \
+ (0x0500 + ((n) * 0x4))
+
#define CSIPHY_DEFAULT_PARAMS 0
#define CSIPHY_LANE_ENABLE 1
#define CSIPHY_SETTLE_CNT_LOWER_BYTE 2
#define CSIPHY_SETTLE_CNT_HIGHER_BYTE 3
#define CSIPHY_DNP_PARAMS 4
#define CSIPHY_2PH_REGS 5
#define CSIPHY_3PH_REGS 6
#define CSIPHY_SKEW_CAL 7
@@ -141,16 +148,17 @@ csiphy_lane_regs lane_regs_sa8775p[] = {
{0x0460, 0xFD, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0464, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x065C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0660, 0xFD, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
};
/* GEN2 1.0 2PH */
+/* 5 entries: clock + 4 lanes */
static const struct
csiphy_lane_regs lane_regs_sdm845[] = {
{0x0004, 0x0C, 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},
{0x0028, 0x00, 0x00, CSIPHY_DNP_PARAMS},
@@ -215,16 +223,82 @@ csiphy_lane_regs lane_regs_sdm845[] = {
{0x0608, 0x00, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{0x060C, 0x00, 0x00, CSIPHY_DNP_PARAMS},
{0x0610, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0638, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0660, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
};
+/* GEN2 1.0 3PH */
+/* 3 entries: 3 lanes (C-PHY) */
+static const struct
+csiphy_lane_regs lane_regs_sdm845_3ph[] = {
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+};
+
/* GEN2 1.1 2PH */
static const struct
csiphy_lane_regs lane_regs_sc8280xp[] = {
{0x0004, 0x0C, 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},
@@ -1166,18 +1240,18 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
struct csiphy_device_regs *regs = csiphy->regs;
u8 settle_cnt;
u8 val;
int i;
switch (csiphy->camss->res->version) {
case CAMSS_845:
if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
- regs->lane_regs = NULL;
- regs->lane_array_size = 0;
+ regs->lane_regs = &lane_regs_sdm845_3ph[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845_3ph);
} else {
regs->lane_regs = &lane_regs_sdm845[0];
regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
}
break;
case CAMSS_2290:
case CAMSS_6150:
if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 C-PHY init
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
Add a PHY configuration sequence for the sdm845 which uses a Qualcomm
Gen 2 version 1.1 CSI-2 PHY.
The PHY can be configured as two phase or three phase in C-PHY or D-PHY
mode. This configuration supports three-phase C-PHY mode.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 78 +++++++++++++++++++++-
1 file changed, 76 insertions(+), 2 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 c76b9c352ea02..d9136e422f45f 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
@@ -45,16 +45,23 @@
#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n) ((offset) + 0x4 * (n))
#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, common_status_offset, n) \
((offset) + (common_status_offset) + 0x4 * (n))
+#define CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(n) \
+ (0x0100 + ((n) * 0x4))
+#define CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(n) \
+ (0x0300 + ((n) * 0x4))
+#define CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(n) \
+ (0x0500 + ((n) * 0x4))
+
#define CSIPHY_DEFAULT_PARAMS 0
#define CSIPHY_LANE_ENABLE 1
#define CSIPHY_SETTLE_CNT_LOWER_BYTE 2
#define CSIPHY_SETTLE_CNT_HIGHER_BYTE 3
#define CSIPHY_DNP_PARAMS 4
#define CSIPHY_2PH_REGS 5
#define CSIPHY_3PH_REGS 6
#define CSIPHY_SKEW_CAL 7
@@ -141,16 +148,17 @@ csiphy_lane_regs lane_regs_sa8775p[] = {
{0x0460, 0xFD, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0464, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x065C, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0660, 0xFD, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
};
/* GEN2 1.0 2PH */
+/* 5 entries: clock + 4 lanes */
static const struct
csiphy_lane_regs lane_regs_sdm845[] = {
{0x0004, 0x0C, 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},
{0x0028, 0x00, 0x00, CSIPHY_DNP_PARAMS},
@@ -215,16 +223,82 @@ csiphy_lane_regs lane_regs_sdm845[] = {
{0x0608, 0x00, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{0x060C, 0x00, 0x00, CSIPHY_DNP_PARAMS},
{0x0610, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0638, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0660, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
};
+/* GEN2 1.0 3PH */
+/* 3 entries: 3 lanes (C-PHY) */
+static const struct
+csiphy_lane_regs lane_regs_sdm845_3ph[] = {
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
+
+};
+
/* GEN2 1.1 2PH */
static const struct
csiphy_lane_regs lane_regs_sc8280xp[] = {
{0x0004, 0x0C, 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},
@@ -1166,18 +1240,18 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
struct csiphy_device_regs *regs = csiphy->regs;
u8 settle_cnt;
u8 val;
int i;
switch (csiphy->camss->res->version) {
case CAMSS_845:
if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
- regs->lane_regs = NULL;
- regs->lane_array_size = 0;
+ regs->lane_regs = &lane_regs_sdm845_3ph[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845_3ph);
} else {
regs->lane_regs = &lane_regs_sdm845[0];
regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
}
break;
case CAMSS_2290:
case CAMSS_6150:
if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 C-PHY init
2026-06-05 13:14 ` David Heidelberg
(?)
@ 2026-06-08 9:00 ` Bryan O'Donoghue
2026-06-08 10:37 ` David Heidelberg
-1 siblings, 1 reply; 28+ messages in thread
From: Bryan O'Donoghue @ 2026-06-08 9:00 UTC (permalink / raw)
To: david, Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Luca Weiss,
Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel
On 05/06/2026 14:14, David Heidelberg via B4 Relay wrote:
> case CAMSS_845:
> if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
> - regs->lane_regs = NULL;
> - regs->lane_array_size = 0;
> + regs->lane_regs = &lane_regs_sdm845_3ph[0];
> + regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845_3ph);
> } else {
> regs->lane_regs = &lane_regs_sdm845[0];
> regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
> }
The nittiest nit pick I can make sorry ...
IMO the default in the code should be DPHY with CPHY the alternative.
Globally that should be true in this code.
---
bod
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 5/8] media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 MIPI CSI-2 C-PHY init
2026-06-08 9:00 ` Bryan O'Donoghue
@ 2026-06-08 10:37 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-08 10:37 UTC (permalink / raw)
To: Bryan O'Donoghue, Robert Foss, Todor Tomov,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel
On 08/06/2026 11:00, Bryan O'Donoghue wrote:
> On 05/06/2026 14:14, David Heidelberg via B4 Relay wrote:
>> case CAMSS_845:
>> if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> - regs->lane_regs = NULL;
>> - regs->lane_array_size = 0;
>> + regs->lane_regs = &lane_regs_sdm845_3ph[0];
>> + regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845_3ph);
>> } else {
>> regs->lane_regs = &lane_regs_sdm845[0];
>> regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
>> }
>
> The nittiest nit pick I can make sorry ...
>
> IMO the default in the code should be DPHY with CPHY the alternative.
Sounds good, thou, the original lane regs, due to being only D-PHY available at
that time, were uncoditional on V4L2_MBUS_CSI2_DPHY (which I assume has to be
set anyway).
I was aiming to keep the original logic (thus else { ... D-PHY }).
Then I would do
if (DPHY)
elseif (CPHY)
if that's desired.
My pref would be another switch statement, but yo dawg, I heard you have switch
statement inside the switch statement.... :)
David
>
> Globally that should be true in this code.
>
> ---
> bod
--
David Heidelberg
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 6/8] media: qcom: camss: csiphy-3ph: Update Gen2 v1.1 MIPI CSI-2 C-PHY init
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
These values should improve C-PHY behaviour. Should match most recent
Qualcomm code.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Suggested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 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 d9136e422f45f..feafe8cc45e02 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
@@ -227,19 +227,19 @@ csiphy_lane_regs lane_regs_sdm845[] = {
{0x0660, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
};
/* GEN2 1.0 3PH */
/* 3 entries: 3 lanes (C-PHY) */
static const struct
csiphy_lane_regs lane_regs_sdm845_3ph[] = {
- {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(23), 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(26), 0xac, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(27), 0xa5, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
@@ -247,19 +247,19 @@ csiphy_lane_regs lane_regs_sdm845_3ph[] = {
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(23), 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(26), 0xac, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(27), 0xa5, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
@@ -267,36 +267,35 @@ csiphy_lane_regs lane_regs_sdm845_3ph[] = {
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(23), 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(26), 0xac, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(27), 0xa5, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
-
};
/* GEN2 1.1 2PH */
static const struct
csiphy_lane_regs lane_regs_sc8280xp[] = {
{0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0034, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 6/8] media: qcom: camss: csiphy-3ph: Update Gen2 v1.1 MIPI CSI-2 C-PHY init
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
These values should improve C-PHY behaviour. Should match most recent
Qualcomm code.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Suggested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
.../media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 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 d9136e422f45f..feafe8cc45e02 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
@@ -227,19 +227,19 @@ csiphy_lane_regs lane_regs_sdm845[] = {
{0x0660, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0664, 0x7F, 0x00, CSIPHY_DEFAULT_PARAMS},
};
/* GEN2 1.0 3PH */
/* 3 entries: 3 lanes (C-PHY) */
static const struct
csiphy_lane_regs lane_regs_sdm845_3ph[] = {
- {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(23), 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(26), 0xac, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(27), 0xa5, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
@@ -247,19 +247,19 @@ csiphy_lane_regs lane_regs_sdm845_3ph[] = {
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN1_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(23), 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(26), 0xac, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(27), 0xa5, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
@@ -267,36 +267,35 @@ csiphy_lane_regs lane_regs_sdm845_3ph[] = {
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN3_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(23), 0x43, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(26), 0xa0, 0x00, CSIPHY_DEFAULT_PARAMS},
- {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(27), 0x25, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(23), 0x63, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(26), 0xac, 0x00, CSIPHY_DEFAULT_PARAMS},
+ {CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(27), 0xa5, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(1), 0x06, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(3), 0x12, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(2), 0x00, 0x00, CSIPHY_SETTLE_CNT_HIGHER_BYTE},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(5), 0x20, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(20), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(6), 0x3e, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(7), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(8), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(9), 0x7f, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(10), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(11), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(17), 0x12, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(24), 0x02, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(51), 0x41, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(25), 0x00, 0x00, CSIPHY_DEFAULT_PARAMS},
{CSIPHY_LN5_CSI_3PH_CTRLn_ADDR(55), 0x51, 0x00, CSIPHY_DEFAULT_PARAMS},
-
};
/* GEN2 1.1 2PH */
static const struct
csiphy_lane_regs lane_regs_sc8280xp[] = {
{0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS},
{0x0034, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS},
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v7 7/8] media: qcom: camss: Account for C-PHY when calculating link frequency
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
Ensure that the link frequency divider correctly accounts for C-PHY
operation. The divider differs between D-PHY and C-PHY, as described
in the MIPI CSI-2 specification.
For more details, see:
https://docs.kernel.org/driver-api/media/tx-rx.html#pixel-rate
Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Cory Keitz <ckeitz@amazon.com>
Tested-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Link: https://docs.kernel.org/driver-api/media/tx-rx.html#pixel-rate
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss-csid.c | 7 +++++--
drivers/media/platform/qcom/camss/camss-csiphy.c | 6 ++----
drivers/media/platform/qcom/camss/camss.c | 18 +++++++++++++++---
drivers/media/platform/qcom/camss/camss.h | 2 +-
4 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index bcc34ac9dd212..7415e811082da 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -534,25 +534,28 @@ const struct csid_format_info *csid_get_fmt_entry(const struct csid_format_info
/*
* csid_set_clock_rates - Calculate and set clock rates on CSID module
* @csiphy: CSID device
*/
static int csid_set_clock_rates(struct csid_device *csid)
{
struct device *dev = csid->camss->dev;
+ struct csiphy_device *csiphy = &csid->camss->csiphy[csid->phy.csiphy_id];
+ struct csiphy_lanes_cfg *lane_cfg = &csiphy->cfg.csi2->lane_cfg;
const struct csid_format_info *fmt;
+
s64 link_freq;
int i, j;
int ret;
fmt = csid_get_fmt_entry(csid->res->formats->formats, csid->res->formats->nformats,
csid->fmt[MSM_CSIPHY_PAD_SINK].code);
- link_freq = camss_get_link_freq(&csid->subdev.entity, fmt->bpp,
- csid->phy.lane_cnt);
+
+ link_freq = camss_get_link_freq(&csid->subdev.entity, fmt->bpp, lane_cfg);
if (link_freq < 0)
link_freq = 0;
for (i = 0; i < csid->nclocks; i++) {
struct camss_clock *clock = &csid->clock[i];
if (!strcmp(clock->name, "csi0") ||
!strcmp(clock->name, "csi1") ||
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 539ac4888b608..f9b1ed79e15de 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -138,19 +138,18 @@ static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
{
struct device *dev = csiphy->camss->dev;
s64 link_freq;
int i, j;
int ret;
u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
- u8 num_lanes = csiphy->cfg.csi2->lane_cfg.num_data;
- link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes);
+ link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, &csiphy->cfg.csi2->lane_cfg);
if (link_freq < 0)
link_freq = 0;
for (i = 0; i < csiphy->nclocks; i++) {
struct camss_clock *clock = &csiphy->clock[i];
if (csiphy->rate_set[i]) {
u64 min_rate = link_freq / 4;
@@ -264,20 +263,19 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
*/
static int csiphy_stream_on(struct csiphy_device *csiphy)
{
struct csiphy_config *cfg = &csiphy->cfg;
s64 link_freq;
u8 lane_mask = csiphy->res->hw_ops->get_lane_mask(&cfg->csi2->lane_cfg);
u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
- u8 num_lanes = csiphy->cfg.csi2->lane_cfg.num_data;
u8 val;
- link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes);
+ link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, &csiphy->cfg.csi2->lane_cfg);
if (link_freq < 0) {
dev_err(csiphy->camss->dev,
"Cannot get CSI2 transmitter's link frequency\n");
return -EINVAL;
}
if (csiphy->base_clk_mux) {
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 072c428e25166..db4e14a84a95f 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -27,16 +27,24 @@
#include <media/v4l2-mc.h>
#include <media/v4l2-fwnode.h>
#include "camss.h"
#define CAMSS_CLOCK_MARGIN_NUMERATOR 105
#define CAMSS_CLOCK_MARGIN_DENOMINATOR 100
+/*
+ * C-PHY encodes data by 16/7 ~ 2.28 bits/symbol
+ * D-PHY doesn't encode data, thus 16/16 = 1 b/s
+ */
+#define CAMSS_COMMON_PHY_DIVIDENT 16
+#define CAMSS_CPHY_DIVISOR 7
+#define CAMSS_DPHY_DIVISOR 16
+
static const struct parent_dev_ops vfe_parent_dev_ops;
static const struct camss_subdev_resources csiphy_res_8x16[] = {
/* CSIPHY0 */
{
.regulators = {},
.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer" },
.clock_rate = { { 0 },
@@ -4618,30 +4626,34 @@ struct media_pad *camss_find_sensor_pad(struct media_entity *entity)
return pad;
}
}
/**
* camss_get_link_freq - Get link frequency from sensor
* @entity: Media entity in the current pipeline
* @bpp: Number of bits per pixel for the current format
- * @lanes: Number of lanes in the link to the sensor
+ * @lane_cfg: CSI2 lane configuration
*
* Return link frequency on success or a negative error code otherwise
*/
s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
- unsigned int lanes)
+ struct csiphy_lanes_cfg *lane_cfg)
{
struct media_pad *sensor_pad;
+ u8 num_lanes = lane_cfg->num_data;
+ bool cphy = lane_cfg->phy_cfg == V4L2_MBUS_CSI2_CPHY;
+ unsigned int div = num_lanes * 2 * (cphy ? CAMSS_CPHY_DIVISOR :
+ CAMSS_DPHY_DIVISOR);
sensor_pad = camss_find_sensor_pad(entity);
if (!sensor_pad)
return -ENODEV;
- return v4l2_get_link_freq(sensor_pad, bpp, 2 * lanes);
+ return v4l2_get_link_freq(sensor_pad, CAMSS_COMMON_PHY_DIVIDENT * bpp, div);
}
/*
* camss_get_pixel_clock - Get pixel clock rate from sensor
* @entity: Media entity in the current pipeline
* @pixel_clock: Received pixel clock value
*
* Return 0 on success or a negative error code otherwise
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 93d691c8ac63b..d65a9b62f7e66 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -164,17 +164,17 @@ struct parent_dev_ops {
};
void camss_add_clock_margin(u64 *rate);
int camss_enable_clocks(int nclocks, struct camss_clock *clock,
struct device *dev);
void camss_disable_clocks(int nclocks, struct camss_clock *clock);
struct media_pad *camss_find_sensor_pad(struct media_entity *entity);
s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
- unsigned int lanes);
+ struct csiphy_lanes_cfg *lane_cfg);
int camss_get_pixel_clock(struct media_entity *entity, u64 *pixel_clock);
int camss_pm_domain_on(struct camss *camss, int id);
void camss_pm_domain_off(struct camss *camss, int id);
int camss_vfe_get(struct camss *camss, int id);
void camss_vfe_put(struct camss *camss, int id);
void camss_delete(struct camss *camss);
void camss_buf_done(struct camss *camss, int hw_id, int port_id);
void camss_reg_update(struct camss *camss, int hw_id,
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 7/8] media: qcom: camss: Account for C-PHY when calculating link frequency
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
Ensure that the link frequency divider correctly accounts for C-PHY
operation. The divider differs between D-PHY and C-PHY, as described
in the MIPI CSI-2 specification.
For more details, see:
https://docs.kernel.org/driver-api/media/tx-rx.html#pixel-rate
Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Cory Keitz <ckeitz@amazon.com>
Tested-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Link: https://docs.kernel.org/driver-api/media/tx-rx.html#pixel-rate
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss-csid.c | 7 +++++--
drivers/media/platform/qcom/camss/camss-csiphy.c | 6 ++----
drivers/media/platform/qcom/camss/camss.c | 18 +++++++++++++++---
drivers/media/platform/qcom/camss/camss.h | 2 +-
4 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index bcc34ac9dd212..7415e811082da 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -534,25 +534,28 @@ const struct csid_format_info *csid_get_fmt_entry(const struct csid_format_info
/*
* csid_set_clock_rates - Calculate and set clock rates on CSID module
* @csiphy: CSID device
*/
static int csid_set_clock_rates(struct csid_device *csid)
{
struct device *dev = csid->camss->dev;
+ struct csiphy_device *csiphy = &csid->camss->csiphy[csid->phy.csiphy_id];
+ struct csiphy_lanes_cfg *lane_cfg = &csiphy->cfg.csi2->lane_cfg;
const struct csid_format_info *fmt;
+
s64 link_freq;
int i, j;
int ret;
fmt = csid_get_fmt_entry(csid->res->formats->formats, csid->res->formats->nformats,
csid->fmt[MSM_CSIPHY_PAD_SINK].code);
- link_freq = camss_get_link_freq(&csid->subdev.entity, fmt->bpp,
- csid->phy.lane_cnt);
+
+ link_freq = camss_get_link_freq(&csid->subdev.entity, fmt->bpp, lane_cfg);
if (link_freq < 0)
link_freq = 0;
for (i = 0; i < csid->nclocks; i++) {
struct camss_clock *clock = &csid->clock[i];
if (!strcmp(clock->name, "csi0") ||
!strcmp(clock->name, "csi1") ||
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 539ac4888b608..f9b1ed79e15de 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -138,19 +138,18 @@ static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
{
struct device *dev = csiphy->camss->dev;
s64 link_freq;
int i, j;
int ret;
u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
- u8 num_lanes = csiphy->cfg.csi2->lane_cfg.num_data;
- link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes);
+ link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, &csiphy->cfg.csi2->lane_cfg);
if (link_freq < 0)
link_freq = 0;
for (i = 0; i < csiphy->nclocks; i++) {
struct camss_clock *clock = &csiphy->clock[i];
if (csiphy->rate_set[i]) {
u64 min_rate = link_freq / 4;
@@ -264,20 +263,19 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
*/
static int csiphy_stream_on(struct csiphy_device *csiphy)
{
struct csiphy_config *cfg = &csiphy->cfg;
s64 link_freq;
u8 lane_mask = csiphy->res->hw_ops->get_lane_mask(&cfg->csi2->lane_cfg);
u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
- u8 num_lanes = csiphy->cfg.csi2->lane_cfg.num_data;
u8 val;
- link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes);
+ link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, &csiphy->cfg.csi2->lane_cfg);
if (link_freq < 0) {
dev_err(csiphy->camss->dev,
"Cannot get CSI2 transmitter's link frequency\n");
return -EINVAL;
}
if (csiphy->base_clk_mux) {
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 072c428e25166..db4e14a84a95f 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -27,16 +27,24 @@
#include <media/v4l2-mc.h>
#include <media/v4l2-fwnode.h>
#include "camss.h"
#define CAMSS_CLOCK_MARGIN_NUMERATOR 105
#define CAMSS_CLOCK_MARGIN_DENOMINATOR 100
+/*
+ * C-PHY encodes data by 16/7 ~ 2.28 bits/symbol
+ * D-PHY doesn't encode data, thus 16/16 = 1 b/s
+ */
+#define CAMSS_COMMON_PHY_DIVIDENT 16
+#define CAMSS_CPHY_DIVISOR 7
+#define CAMSS_DPHY_DIVISOR 16
+
static const struct parent_dev_ops vfe_parent_dev_ops;
static const struct camss_subdev_resources csiphy_res_8x16[] = {
/* CSIPHY0 */
{
.regulators = {},
.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer" },
.clock_rate = { { 0 },
@@ -4618,30 +4626,34 @@ struct media_pad *camss_find_sensor_pad(struct media_entity *entity)
return pad;
}
}
/**
* camss_get_link_freq - Get link frequency from sensor
* @entity: Media entity in the current pipeline
* @bpp: Number of bits per pixel for the current format
- * @lanes: Number of lanes in the link to the sensor
+ * @lane_cfg: CSI2 lane configuration
*
* Return link frequency on success or a negative error code otherwise
*/
s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
- unsigned int lanes)
+ struct csiphy_lanes_cfg *lane_cfg)
{
struct media_pad *sensor_pad;
+ u8 num_lanes = lane_cfg->num_data;
+ bool cphy = lane_cfg->phy_cfg == V4L2_MBUS_CSI2_CPHY;
+ unsigned int div = num_lanes * 2 * (cphy ? CAMSS_CPHY_DIVISOR :
+ CAMSS_DPHY_DIVISOR);
sensor_pad = camss_find_sensor_pad(entity);
if (!sensor_pad)
return -ENODEV;
- return v4l2_get_link_freq(sensor_pad, bpp, 2 * lanes);
+ return v4l2_get_link_freq(sensor_pad, CAMSS_COMMON_PHY_DIVIDENT * bpp, div);
}
/*
* camss_get_pixel_clock - Get pixel clock rate from sensor
* @entity: Media entity in the current pipeline
* @pixel_clock: Received pixel clock value
*
* Return 0 on success or a negative error code otherwise
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 93d691c8ac63b..d65a9b62f7e66 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -164,17 +164,17 @@ struct parent_dev_ops {
};
void camss_add_clock_margin(u64 *rate);
int camss_enable_clocks(int nclocks, struct camss_clock *clock,
struct device *dev);
void camss_disable_clocks(int nclocks, struct camss_clock *clock);
struct media_pad *camss_find_sensor_pad(struct media_entity *entity);
s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
- unsigned int lanes);
+ struct csiphy_lanes_cfg *lane_cfg);
int camss_get_pixel_clock(struct media_entity *entity, u64 *pixel_clock);
int camss_pm_domain_on(struct camss *camss, int id);
void camss_pm_domain_off(struct camss *camss, int id);
int camss_vfe_get(struct camss *camss, int id);
void camss_vfe_put(struct camss *camss, int id);
void camss_delete(struct camss *camss);
void camss_buf_done(struct camss *camss, int hw_id, int port_id);
void camss_reg_update(struct camss *camss, int hw_id,
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 7/8] media: qcom: camss: Account for C-PHY when calculating link frequency
2026-06-05 13:14 ` David Heidelberg
(?)
@ 2026-06-05 14:00 ` sashiko-bot
-1 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-05 14:00 UTC (permalink / raw)
To: David Heidelberg via B4 Relay; +Cc: linux-media
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Deterministic NULL pointer dereference when calculating link frequency for the CSID linked to the internal Test Pattern Generator (TPG).
- [High] The C-PHY link frequency calculation inverses the symbol rate ratio, resulting in a severely inflated link frequency.
--
commit 0a60c2346d24cbca656cf347fe3da5342c1b9381
Author: David Heidelberg <david@ixit.cz>
media: qcom: camss: Account for C-PHY when calculating link frequency
Ensure that the link frequency divider correctly accounts for C-PHY
operation. The divider differs between D-PHY and C-PHY, as described
in the MIPI CSI-2 specification.
Link: https://docs.kernel.org/driver-api/media/tx-rx.html#pixel-rate
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index bcc34ac9dd212..7415e811082da 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -539,15 +539,18 @@ const struct csid_format_info *csid_get_fmt_entry(const struct csid_format_info
> static int csid_set_clock_rates(struct csid_device *csid)
> {
> struct device *dev = csid->camss->dev;
> + struct csiphy_device *csiphy = &csid->camss->csiphy[csid->phy.csiphy_id];
> + struct csiphy_lanes_cfg *lane_cfg = &csiphy->cfg.csi2->lane_cfg;
[Severity: High]
Will this cause a NULL pointer dereference when the CSID is linked to the
internal Test Pattern Generator (TPG)?
When the CSID is linked to the internal TPG, csid->phy.csiphy_id is set to
the TPG index, and the pipeline does not have a sensor bound. If
csiphy->cfg.csi2 is NULL, lane_cfg evaluates to an invalid pointer (an
offset from NULL).
This invalid pointer is then passed to camss_get_link_freq(), which
unconditionally dereferences lane_cfg->num_data before verifying if a sensor
pad actually exists. This would trigger a kernel panic. Before this patch,
the code passed csid->phy.lane_cnt by value, safely returning -ENODEV for
TPG and gracefully defaulting to a zero fallback frequency.
> const struct csid_format_info *fmt;
> +
> s64 link_freq;
> int i, j;
> int ret;
>
> fmt = csid_get_fmt_entry(csid->res->formats->formats, csid->res->formats->nformats,
> csid->fmt[MSM_CSIPHY_PAD_SINK].code);
> - link_freq = camss_get_link_freq(&csid->subdev.entity, fmt->bpp,
> - csid->phy.lane_cnt);
> +
> + link_freq = camss_get_link_freq(&csid->subdev.entity, fmt->bpp, lane_cfg);
[ ... ]
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 072c428e25166..db4e14a84a95f 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
[ ... ]
> @@ -4623,20 +4631,24 @@ struct media_pad *camss_find_sensor_pad(struct media_entity *entity)
> * camss_get_link_freq - Get link frequency from sensor
> * @entity: Media entity in the current pipeline
> * @bpp: Number of bits per pixel for the current format
> - * @lanes: Number of lanes in the link to the sensor
> + * @lane_cfg: CSI2 lane configuration
> *
> * Return link frequency on success or a negative error code otherwise
> */
> s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
> - unsigned int lanes)
> + struct csiphy_lanes_cfg *lane_cfg)
> {
> struct media_pad *sensor_pad;
> + u8 num_lanes = lane_cfg->num_data;
> + bool cphy = lane_cfg->phy_cfg == V4L2_MBUS_CSI2_CPHY;
> + unsigned int div = num_lanes * 2 * (cphy ? CAMSS_CPHY_DIVISOR :
> + CAMSS_DPHY_DIVISOR);
>
> sensor_pad = camss_find_sensor_pad(entity);
> if (!sensor_pad)
> return -ENODEV;
>
> - return v4l2_get_link_freq(sensor_pad, bpp, 2 * lanes);
> + return v4l2_get_link_freq(sensor_pad, CAMSS_COMMON_PHY_DIVIDENT * bpp, div);
[Severity: High]
Does this calculation invert the C-PHY link frequency ratio?
According to the MIPI C-PHY specification and V4L2 documentation, 16 bits encode
into 7 symbols, meaning the symbol rate (link frequency) should be:
(pixel_rate * bpp * 7) / (16 * num_lanes).
Here, the multiplier passed to v4l2_get_link_freq() is (16 * bpp), and the
divisor is (num_lanes * 2 * 7) for C-PHY. This effectively multiplies by 16
and divides by 14, instead of multiplying by 7 and dividing by 16.
This logic appears to inflate the calculated link frequency for C-PHY by a
factor of roughly 2.6x, which could lead to pixel clock configuration failures
or physical hardware malfunction.
> }
>
> /*
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-qcom-cphy-v7-0-426c37e9008f@ixit.cz?part=7
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 8/8] media: qcom: camss: Enable C-PHY where available
2026-06-05 13:14 ` David Heidelberg
@ 2026-06-05 13:14 ` David Heidelberg
-1 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
From: David Heidelberg <david@ixit.cz>
After all the changes done we can now safely enable C-PHY for a SoC
where it's available.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index db4e14a84a95f..555c53343a1e9 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4761,21 +4761,21 @@ static int camss_parse_endpoint_node(struct device *dev,
struct v4l2_fwnode_endpoint vep = { { 0 } };
unsigned int i;
int ret;
ret = v4l2_fwnode_endpoint_parse(ep, &vep);
if (ret)
return ret;
- /*
- * Most SoCs support both D-PHY and C-PHY standards, but currently only
- * D-PHY is supported in the driver.
- */
- if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
+ switch (vep.bus_type) {
+ case V4L2_MBUS_CSI2_CPHY:
+ case V4L2_MBUS_CSI2_DPHY:
+ break;
+ default:
dev_err(dev, "Unsupported bus type %d\n", vep.bus_type);
return -EINVAL;
}
csd->interface.csiphy_id = vep.base.port;
mipi_csi2 = &vep.bus.mipi_csi2;
lncfg->num_data = mipi_csi2->num_data_lanes;
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v7 8/8] media: qcom: camss: Enable C-PHY where available
@ 2026-06-05 13:14 ` David Heidelberg
0 siblings, 0 replies; 28+ messages in thread
From: David Heidelberg @ 2026-06-05 13:14 UTC (permalink / raw)
To: Robert Foss, Todor Tomov, Bryan O'Donoghue,
Bryan O'Donoghue, Vladimir Zapolskiy, Mauro Carvalho Chehab,
Luca Weiss, Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel,
David Heidelberg
After all the changes done we can now safely enable C-PHY for a SoC
where it's available.
Acked-by: Cory Keitz <ckeitz@amazon.com>
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/media/platform/qcom/camss/camss.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index db4e14a84a95f..555c53343a1e9 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4761,21 +4761,21 @@ static int camss_parse_endpoint_node(struct device *dev,
struct v4l2_fwnode_endpoint vep = { { 0 } };
unsigned int i;
int ret;
ret = v4l2_fwnode_endpoint_parse(ep, &vep);
if (ret)
return ret;
- /*
- * Most SoCs support both D-PHY and C-PHY standards, but currently only
- * D-PHY is supported in the driver.
- */
- if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
+ switch (vep.bus_type) {
+ case V4L2_MBUS_CSI2_CPHY:
+ case V4L2_MBUS_CSI2_DPHY:
+ break;
+ default:
dev_err(dev, "Unsupported bus type %d\n", vep.bus_type);
return -EINVAL;
}
csd->interface.csiphy_id = vep.base.port;
mipi_csi2 = &vep.bus.mipi_csi2;
lncfg->num_data = mipi_csi2->num_data_lanes;
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v7 0/8] media: camss: Add support for C-PHY configuration on Qualcomm platforms
2026-06-05 13:14 ` David Heidelberg
` (8 preceding siblings ...)
(?)
@ 2026-06-08 8:56 ` Bryan O'Donoghue
-1 siblings, 0 replies; 28+ messages in thread
From: Bryan O'Donoghue @ 2026-06-08 8:56 UTC (permalink / raw)
To: david, Robert Foss, Todor Tomov, Bryan O'Donoghue,
Vladimir Zapolskiy, Mauro Carvalho Chehab, Luca Weiss,
Petr Hodina, Dr. Git, Cory Keitz, Loic Poulain
Cc: Frank Li, Konrad Dybcio, Kieran Bingham, Sakari Ailus,
linux-media, linux-arm-msm, linux-kernel, phone-devel
On 05/06/2026 14:14, David Heidelberg via B4 Relay wrote:
> Note: WIP tag added, as not everything from the previous review round has
> been addressed.
>
> # Short summary
>
> This patch series extends the Qualcomm CAMSS (Camera Subsystem),
> including CSID and CSIPHY components, to support C-PHY mode configuration.
>
> # Background and motivation
>
> Modern smartphone cameras increasingly rely on MIPI C-PHY rather than
> D-PHY, thanks to its higher data throughput and signal efficiency.
> As a result, many OEMs adopt C-PHY interfaces for main (rear) cameras on
> Qualcomm-based devices.
>
> Until now, mainline Linux lacked C-PHY configuration support for Qualcomm
> chipsets, preventing bring-up of primary camera sensors on several
> Snapdragon platforms. This series closes that gap.
>
> - Introduces C-PHY configuration support for the CAMSS driver stack,
> covering both CSID and CSIPHY blocks.
> - Successfully enables C-PHY operation on the Snapdragon 845 platform.
> - Tested on OnePlus 6 and 6T phones running mainline Linux,
> using the Sony IMX519 main camera sensor.
> - The new configuration allows other chipsets versionsto enable C-PHY by
> simply adding corresponding sensor driver support and csiphy
> initialization data, following the example set for sdm845.
>
> With this patch series, mainline Linux gains working C-PHY support for
> Snapdragon 845, paving the way for improved main camera functionality
> across many Qualcomm-based devices. The groundwork also simplifies
> future enablement efforts for additional SoCs and sensors.
>
> Until merged, the series will be also available at:
> https://codeberg.org/sdm845/linux/commits/branch/b4/qcom-cphy
>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
> Changes in v7:
> - Move logic out of switch. (Frank)
> - Change csid cphy knob to phy_sel using enum. (Bryan, Frank)
> - Link to v6: https://lore.kernel.org/r/20260603-qcom-cphy-v6-0-e50de0b557a8@ixit.cz
So I was asking you previously about a dts to support this in some way
in effect to prove it works.
I had a meeting with some qcom guys on Friday and the informed me they
have based work on top of yours, which is inference enough that it works.
Some Tested-by: from others interested in this work, would be greatly
appreciated at this stage.
---
bod
^ permalink raw reply [flat|nested] 28+ messages in thread