[PATCH WIP v4 8/9] media: qcom: camss: csiphy-3ph: C-PHY needs own lane configuration

David Heidelberg via B4 Relay posted 9 patches 1 month, 1 week ago
[PATCH WIP v4 8/9] media: qcom: camss: csiphy-3ph: C-PHY needs own lane configuration
Posted by David Heidelberg via B4 Relay 1 month, 1 week ago
From: David Heidelberg <david@ixit.cz>

Catch when C-PHY configuration gets used on SoC with CAMSS missing C-PHY
configuration lane registers.

Hopefully this check will disappear as these lane regs gets populated.

--
@bod
Proliferating special cases in switch statements on a per-SoC basis is
verboten.

Please find another way to do this, you already have a bool to indicate
cphy in struct csid_phy_config {} so at some level CAMSS already has a
bool to indicate what to do.

Please make that logic accessible to logical consumers throughout,
in this case the CPHY code.
--

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

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 e48f5c1290173..2164647211c8d 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
@@ -1221,6 +1221,22 @@ static int csiphy_lanes_enable(struct csiphy_device *csiphy,
 	u8 val;
 	int i;
 
+	if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
+		switch (csiphy->camss->res->version) {
+		case CAMSS_2290:
+		case CAMSS_8280XP:
+		case CAMSS_X1E80100:
+		case CAMSS_8550:
+		case CAMSS_8650:
+		case CAMSS_8300:
+		case CAMSS_8775P:
+			dev_err(dev, "Missing lane_regs definition for C-PHY\n");
+			return -EINVAL;
+		default:
+			break;
+		}
+	}
+
 	switch (csiphy->camss->res->version) {
 	case CAMSS_845:
 		if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {

-- 
2.51.0
Re: [PATCH WIP v4 8/9] media: qcom: camss: csiphy-3ph: C-PHY needs own lane configuration
Posted by Bryan O'Donoghue 1 month, 1 week ago
On 01/03/2026 00:51, David Heidelberg via B4 Relay wrote:
> @bod
> Proliferating special cases in switch statements on a per-SoC basis is
> verboten.

Hmm, your ideas are intriguing to me, and I wish to subscribe to your 
newsletter.

---
bod
Re: [PATCH WIP v4 8/9] media: qcom: camss: csiphy-3ph: C-PHY needs own lane configuration
Posted by Bryan O'Donoghue 1 week, 4 days ago
On 03/03/2026 09:59, Bryan O'Donoghue wrote:
> On 01/03/2026 00:51, David Heidelberg via B4 Relay wrote:
>> @bod
>> Proliferating special cases in switch statements on a per-SoC basis is
>> verboten.
> 
> Hmm, your ideas are intriguing to me, and I wish to subscribe to your
> newsletter.
> 
> ---
> bod
> 

@David.

I'm realising my Simpson's reference probably isn't actionable 
development feedback ;)

How about embeddding a pointer to either a DPHY or CPHY init sequence 
somewhere and checking that pointer ?

If it the CPHY init sequence/table in this case, is NULL return 
-EOPTNOTSUPP.

That way instead of constantly extending a switch for each new SoC we 
enumerate a pointer and check that pointer's validity.

Then we have nice clean code which just checks the value of a pointer, 
instead of an ever-growing list of SoCs in a switch.

---
bod