linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] media: qcom: camss: add support for csiphy devices
@ 2025-06-12  1:15 Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 01/10] media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put() Vladimir Zapolskiy
                   ` (9 more replies)
  0 siblings, 10 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

The changeset introduces an initial version of long awaited CSIPHY
devices, which is agreed to be used along with any new added CAMSS
platform support.

Any platform and board dts changes are not intended to be added into
the series, however the changeset includes sm8250.dtsi and RB5 vision
mezzanine dtso changes for reference and testing purposes.

The changeset is based on top of this one:
* https://lore.kernel.org/all/20250513142353.2572563-1-vladimir.zapolskiy@linaro.org/

The changeset includes a new generic CSIPHY dt bindings evolved from
an RFC one, which has been submitted under phy folder originally:
* https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/

Known limitations:
* There is no regression with old dt bindings, however under the new
  scheme of CSIPHY to CSID link a TPG may not work expectedly,
* to preserve backwards compatibility all CSIPHY resources are taken
  from CAMSS device tree node, and it's a subject for a future change.

Vladimir Zapolskiy (10):
  media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put()
  media: qcom: camss: remove subdev resource argument from msm_csiphy_subdev_init()
  media: qcom: camss: csiphy: simplify arguments of lanes_enable and lanes_disable
  media: qcom: camss: populate CAMSS children subdevices of CSIPHY IPs
  media: qcom: camss: unwrap platform driver registration
  media: qcom: camss: export camss_parse_endpoint_node() to csiphy
  media: qcom: camss: csiphy: probe any present children CSIPHY subdevices
  dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  [RFT] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: switch to new CSIPHY scheme

 .../bindings/media/qcom,csiphy.yaml           |  96 +++++++
 .../qcom/qrb5165-rb5-vision-mezzanine.dtso    |  18 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |  14 +
 .../qcom/camss/camss-csiphy-2ph-1-0.c         |  10 +-
 .../qcom/camss/camss-csiphy-3ph-1-0.c         |  11 +-
 .../media/platform/qcom/camss/camss-csiphy.c  | 258 +++++++++++++++++-
 .../media/platform/qcom/camss/camss-csiphy.h  |  13 +-
 drivers/media/platform/qcom/camss/camss.c     |  98 ++++++-
 drivers/media/platform/qcom/camss/camss.h     |   6 +-
 9 files changed, 480 insertions(+), 44 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,csiphy.yaml

-- 
2.49.0


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

* [PATCH 01/10] media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put()
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  7:31   ` Bryan O'Donoghue
  2025-06-12  1:15 ` [PATCH 02/10] media: qcom: camss: remove subdev resource argument from msm_csiphy_subdev_init() Vladimir Zapolskiy
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

Two intended to be helpers camss_vfe_get()/camss_vfe_put() got their
declarations in commit b1e6eef535df ("media: qcom: camss: Decouple VFE
from CSID"), but the correspondent functions haven't beed even added.

Remove the unused declarations.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 1d0f83e4a2c9..99831846ebb5 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -160,8 +160,6 @@ s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
 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.49.0


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

* [PATCH 02/10] media: qcom: camss: remove subdev resource argument from msm_csiphy_subdev_init()
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 01/10] media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put() Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  7:37   ` Bryan O'Donoghue
  2025-06-12  1:15 ` [PATCH 03/10] media: qcom: camss: csiphy: simplify arguments of lanes_enable and lanes_disable Vladimir Zapolskiy
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

The removed argument is directly and unambiguously derived from other
ones.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csiphy.c | 6 +++---
 drivers/media/platform/qcom/camss/camss-csiphy.h | 5 +----
 drivers/media/platform/qcom/camss/camss.c        | 1 -
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 2de97f58f9ae..1ba3fc2e33ac 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -569,16 +569,16 @@ static bool csiphy_match_clock_name(const char *clock_name, const char *format,
 
 /*
  * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
+ * @camss: CAMSS device
  * @csiphy: CSIPHY device
- * @res: CSIPHY module resources table
  * @id: CSIPHY module id
  *
  * Return 0 on success or a negative error code otherwise
  */
 int msm_csiphy_subdev_init(struct camss *camss,
-			   struct csiphy_device *csiphy,
-			   const struct camss_subdev_resources *res, u8 id)
+			   struct csiphy_device *csiphy, u8 id)
 {
+	const struct camss_subdev_resources *res = &camss->res->csiphy_res[id];
 	struct device *dev = camss->dev;
 	struct platform_device *pdev = to_platform_device(dev);
 	int i, j;
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 895f80003c44..d82dafd1d270 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -113,11 +113,8 @@ struct csiphy_device {
 	struct csiphy_device_regs *regs;
 };
 
-struct camss_subdev_resources;
-
 int msm_csiphy_subdev_init(struct camss *camss,
-			   struct csiphy_device *csiphy,
-			   const struct camss_subdev_resources *res, u8 id);
+			   struct csiphy_device *csiphy, u8 id);
 
 int msm_csiphy_register_entity(struct csiphy_device *csiphy,
 			       struct v4l2_device *v4l2_dev);
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 0d05f52a6e92..695f113472a5 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -3074,7 +3074,6 @@ static int camss_init_subdevices(struct camss *camss)
 
 	for (i = 0; i < camss->res->csiphy_num; i++) {
 		ret = msm_csiphy_subdev_init(camss, &camss->csiphy[i],
-					     &res->csiphy_res[i],
 					     res->csiphy_res[i].csiphy.id);
 		if (ret < 0) {
 			dev_err(camss->dev,
-- 
2.49.0


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

* [PATCH 03/10] media: qcom: camss: csiphy: simplify arguments of lanes_enable and lanes_disable
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 01/10] media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put() Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 02/10] media: qcom: camss: remove subdev resource argument from msm_csiphy_subdev_init() Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  8:02   ` Bryan O'Donoghue
  2025-06-12  1:15 ` [PATCH 04/10] media: qcom: camss: populate CAMSS children subdevices of CSIPHY IPs Vladimir Zapolskiy
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

In some of .lanes_enable and .lanes_disable functions the second argument
of csiphy_config type is either unused or it can be derived from the
main function argument, this lets to remove it from the list of arguments.

Apart of being the simplification the change is needed for further updates
to CSIPHY part of the CAMSS driver to get CSIPHY combo mode feature and
a related to it management of non-statically assigned CSIPHY media pads.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 .../media/platform/qcom/camss/camss-csiphy-2ph-1-0.c  | 10 ++++------
 .../media/platform/qcom/camss/camss-csiphy-3ph-1-0.c  | 11 ++++-------
 drivers/media/platform/qcom/camss/camss-csiphy.c      |  4 ++--
 drivers/media/platform/qcom/camss/camss-csiphy.h      |  4 +---
 4 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c
index 9d67e7fa6366..d9735f61fffc 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c
@@ -95,10 +95,9 @@ static u8 csiphy_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
 }
 
 static void csiphy_lanes_enable(struct csiphy_device *csiphy,
-				struct csiphy_config *cfg,
 				s64 link_freq, u8 lane_mask)
 {
-	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
+	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
 	u8 settle_cnt;
 	u8 val, l = 0;
 	int i = 0;
@@ -114,7 +113,7 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
 	val |= lane_mask << 1;
 	writel_relaxed(val, csiphy->base + CAMSS_CSI_PHY_GLBL_PWR_CFG);
 
-	val = cfg->combo_mode << 4;
+	val = csiphy->cfg.combo_mode << 4;
 	writel_relaxed(val, csiphy->base + CAMSS_CSI_PHY_GLBL_RESET);
 
 	for (i = 0; i <= c->num_data; i++) {
@@ -134,10 +133,9 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
 	}
 }
 
-static void csiphy_lanes_disable(struct csiphy_device *csiphy,
-				 struct csiphy_config *cfg)
+static void csiphy_lanes_disable(struct csiphy_device *csiphy)
 {
-	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
+	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
 	u8 l = 0;
 	int i = 0;
 
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 f732a76de93e..69d95bfeb9d2 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
@@ -638,10 +638,9 @@ static u8 csiphy_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
 }
 
 static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,
-				     struct csiphy_config *cfg,
 				     u8 settle_cnt)
 {
-	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
+	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
 	int i, l = 0;
 	u8 val;
 
@@ -758,10 +757,9 @@ static bool csiphy_is_gen2(u32 version)
 }
 
 static void csiphy_lanes_enable(struct csiphy_device *csiphy,
-				struct csiphy_config *cfg,
 				s64 link_freq, u8 lane_mask)
 {
-	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
+	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
 	struct csiphy_device_regs *regs = csiphy->regs;
 	u8 settle_cnt;
 	u8 val;
@@ -791,7 +789,7 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
 	if (csiphy_is_gen2(csiphy->camss->res->version))
 		csiphy_gen2_config_lanes(csiphy, settle_cnt);
 	else
-		csiphy_gen1_config_lanes(csiphy, cfg, settle_cnt);
+		csiphy_gen1_config_lanes(csiphy, settle_cnt);
 
 	/* IRQ_MASK registers - disable all interrupts */
 	for (i = 11; i < 22; i++) {
@@ -800,8 +798,7 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
 	}
 }
 
-static void csiphy_lanes_disable(struct csiphy_device *csiphy,
-				 struct csiphy_config *cfg)
+static void csiphy_lanes_disable(struct csiphy_device *csiphy)
 {
 	struct csiphy_device_regs *regs = csiphy->regs;
 
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 1ba3fc2e33ac..f561811b7617 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -295,7 +295,7 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
 		wmb();
 	}
 
-	csiphy->res->hw_ops->lanes_enable(csiphy, cfg, link_freq, lane_mask);
+	csiphy->res->hw_ops->lanes_enable(csiphy, link_freq, lane_mask);
 
 	return 0;
 }
@@ -308,7 +308,7 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
  */
 static void csiphy_stream_off(struct csiphy_device *csiphy)
 {
-	csiphy->res->hw_ops->lanes_disable(csiphy, &csiphy->cfg);
+	csiphy->res->hw_ops->lanes_disable(csiphy);
 }
 
 
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index d82dafd1d270..3b73248f1364 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -72,10 +72,8 @@ struct csiphy_hw_ops {
 				struct device *dev);
 	void (*reset)(struct csiphy_device *csiphy);
 	void (*lanes_enable)(struct csiphy_device *csiphy,
-			     struct csiphy_config *cfg,
 			     s64 link_freq, u8 lane_mask);
-	void (*lanes_disable)(struct csiphy_device *csiphy,
-			      struct csiphy_config *cfg);
+	void (*lanes_disable)(struct csiphy_device *csiphy);
 	irqreturn_t (*isr)(int irq, void *dev);
 	int (*init)(struct csiphy_device *csiphy);
 };
-- 
2.49.0


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

* [PATCH 04/10] media: qcom: camss: populate CAMSS children subdevices of CSIPHY IPs
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
                   ` (2 preceding siblings ...)
  2025-06-12  1:15 ` [PATCH 03/10] media: qcom: camss: csiphy: simplify arguments of lanes_enable and lanes_disable Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 05/10] media: qcom: camss: unwrap platform driver registration Vladimir Zapolskiy
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

To add a number of CSIPHY devices under CAMSS device tree node populate
the children devices, if they are present, and exclude handling of such
discovered CSIPHY devices in runtime from the CAMSS device driver.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 .../media/platform/qcom/camss/camss-csiphy.h  |  1 +
 drivers/media/platform/qcom/camss/camss.c     | 77 ++++++++++++++++++-
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 3b73248f1364..f092b7ff2f26 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -91,6 +91,7 @@ struct csiphy_device_regs {
 };
 
 struct csiphy_device {
+	struct device *dev;
 	struct camss *camss;
 	u8 id;
 	struct v4l2_subdev subdev;
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 695f113472a5..71a37447e17a 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -16,6 +16,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
+#include <linux/of_platform.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
 #include <linux/slab.h>
@@ -3059,6 +3060,47 @@ static int camss_parse_ports(struct camss *camss)
 	return ret;
 }
 
+static void camss_csiphy_put(struct camss *camss)
+{
+	unsigned int i;
+
+	for (i = 0; i < camss->res->csiphy_num; i++)
+		put_device(camss->csiphy[i].dev);
+}
+
+static void camss_csiphy_get(struct camss *camss)
+{
+	struct device_node *camss_node = dev_of_node(camss->dev);
+	unsigned int i;
+	int phys;
+
+	phys = of_count_phandle_with_args(camss_node, "phys", NULL);
+	if (phys < 0)
+		return;
+
+	if (phys != camss->res->csiphy_num)
+		dev_info(camss->dev, "Number of phys is unexpected\n");
+
+	for (i = 0; i < camss->res->csiphy_num; i++) {
+		struct platform_device *phy_pdev;
+		struct device_node *phy_node;
+
+		phy_node = of_parse_phandle(camss_node, "phys", i);
+		if (!phy_node ||
+		    !of_node_name_eq(phy_node, "phy") ||
+		    !of_device_is_available(phy_node))
+			continue;
+
+		/* At this point it shall be possible to reference the phy */
+		phy_pdev = of_find_device_by_node(phy_node);
+		of_node_put(phy_node);
+		if (!phy_pdev)
+			continue;
+
+		camss->csiphy[i].dev = &phy_pdev->dev;
+	}
+}
+
 /*
  * camss_init_subdevices - Initialize subdev structures and resources
  * @camss: CAMSS device
@@ -3073,6 +3115,10 @@ static int camss_init_subdevices(struct camss *camss)
 	int ret;
 
 	for (i = 0; i < camss->res->csiphy_num; i++) {
+		/* Initialize only non-registered CSIPHY subdevices */
+		if (camss->csiphy[i].dev)
+			continue;
+
 		ret = msm_csiphy_subdev_init(camss, &camss->csiphy[i],
 					     res->csiphy_res[i].csiphy.id);
 		if (ret < 0) {
@@ -3155,6 +3201,9 @@ static int camss_link_entities(struct camss *camss)
 	int ret;
 
 	for (i = 0; i < camss->res->csiphy_num; i++) {
+		if (camss->csiphy[i].dev)
+			continue;
+
 		for (j = 0; j < camss->res->csid_num; j++) {
 			ret = media_create_pad_link(&camss->csiphy[i].subdev.entity,
 						    MSM_CSIPHY_PAD_SRC,
@@ -3265,6 +3314,9 @@ static int camss_register_entities(struct camss *camss)
 	int ret;
 
 	for (i = 0; i < camss->res->csiphy_num; i++) {
+		if (camss->csiphy[i].dev)
+			continue;
+
 		ret = msm_csiphy_register_entity(&camss->csiphy[i],
 						 &camss->v4l2_dev);
 		if (ret < 0) {
@@ -3321,7 +3373,8 @@ static int camss_register_entities(struct camss *camss)
 	i = camss->res->csiphy_num;
 err_reg_csiphy:
 	for (i--; i >= 0; i--)
-		msm_csiphy_unregister_entity(&camss->csiphy[i]);
+		if (!camss->csiphy[i].dev)
+			msm_csiphy_unregister_entity(&camss->csiphy[i]);
 
 	return ret;
 }
@@ -3337,7 +3390,8 @@ static void camss_unregister_entities(struct camss *camss)
 	unsigned int i;
 
 	for (i = 0; i < camss->res->csiphy_num; i++)
-		msm_csiphy_unregister_entity(&camss->csiphy[i]);
+		if (!camss->csiphy[i].dev)
+			msm_csiphy_unregister_entity(&camss->csiphy[i]);
 
 	for (i = 0; i < camss->res->csid_num; i++)
 		msm_csid_unregister_entity(&camss->csid[i]);
@@ -3379,6 +3433,9 @@ static int camss_subdev_notifier_complete(struct v4l2_async_notifier *async)
 		if (!csiphy)
 			continue;
 
+		if (csiphy->dev)
+			continue;
+
 		input = &csiphy->subdev.entity;
 		sensor = &sd->entity;
 
@@ -3585,13 +3642,22 @@ static int camss_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = devm_of_platform_populate(camss->dev);
+	if (ret) {
+		dev_err(camss->dev,
+			"Failed to populate camss children devices: %d\n", ret);
+		goto err_genpd_cleanup;
+	}
+
+	camss_csiphy_get(camss);
+
 	ret = camss_init_subdevices(camss);
 	if (ret < 0)
-		goto err_genpd_cleanup;
+		goto err_csiphy_put;
 
 	ret = dma_set_mask_and_coherent(dev, 0xffffffff);
 	if (ret)
-		goto err_genpd_cleanup;
+		goto err_csiphy_put;
 
 	camss->media_dev.dev = camss->dev;
 	strscpy(camss->media_dev.model, "Qualcomm Camera Subsystem",
@@ -3648,6 +3714,8 @@ static int camss_probe(struct platform_device *pdev)
 	pm_runtime_disable(dev);
 err_media_device_cleanup:
 	media_device_cleanup(&camss->media_dev);
+err_csiphy_put:
+	camss_csiphy_put(camss);
 err_genpd_cleanup:
 	camss_genpd_cleanup(camss);
 
@@ -3680,6 +3748,7 @@ static void camss_remove(struct platform_device *pdev)
 	if (atomic_read(&camss->ref_count) == 0)
 		camss_delete(camss);
 
+	camss_csiphy_put(camss);
 	camss_genpd_cleanup(camss);
 }
 
-- 
2.49.0


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

* [PATCH 05/10] media: qcom: camss: unwrap platform driver registration
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
                   ` (3 preceding siblings ...)
  2025-06-12  1:15 ` [PATCH 04/10] media: qcom: camss: populate CAMSS children subdevices of CSIPHY IPs Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 06/10] media: qcom: camss: export camss_parse_endpoint_node() to csiphy Vladimir Zapolskiy
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

To be able to register new CSIPHY device drivers unfold
module_platform_driver() into module_init()/module_exit().

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 71a37447e17a..e03308d7a366 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -3953,7 +3953,17 @@ static struct platform_driver qcom_camss_driver = {
 	},
 };
 
-module_platform_driver(qcom_camss_driver);
+static int __init qcom_camss_init(void)
+{
+	return platform_driver_register(&qcom_camss_driver);
+}
+module_init(qcom_camss_init);
+
+static void __exit qcom_camss_exit(void)
+{
+	platform_driver_unregister(&qcom_camss_driver);
+}
+module_exit(qcom_camss_exit);
 
 MODULE_ALIAS("platform:qcom-camss");
 MODULE_DESCRIPTION("Qualcomm Camera Subsystem driver");
-- 
2.49.0


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

* [PATCH 06/10] media: qcom: camss: export camss_parse_endpoint_node() to csiphy
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
                   ` (4 preceding siblings ...)
  2025-06-12  1:15 ` [PATCH 05/10] media: qcom: camss: unwrap platform driver registration Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 07/10] media: qcom: camss: csiphy: probe any present children CSIPHY subdevices Vladimir Zapolskiy
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

Export the function to reuse the remote endpoint node helper function
from a CSIPHY driver.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 6 +++---
 drivers/media/platform/qcom/camss/camss.h | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index e03308d7a366..40bb20bbe8b4 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -2981,9 +2981,9 @@ static const struct parent_dev_ops vfe_parent_dev_ops = {
  *
  * Return 0 on success or a negative error code on failure
  */
-static int camss_parse_endpoint_node(struct device *dev,
-				     struct fwnode_handle *ep,
-				     struct camss_async_subdev *csd)
+int camss_parse_endpoint_node(struct device *dev,
+			      struct fwnode_handle *ep,
+			      struct camss_async_subdev *csd)
 {
 	struct csiphy_lanes_cfg *lncfg = &csd->interface.csi2.lane_cfg;
 	struct v4l2_mbus_config_mipi_csi2 *mipi_csi2;
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 99831846ebb5..c3eedeb87ddc 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -14,6 +14,7 @@
 #include <linux/types.h>
 #include <media/v4l2-async.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 #include <media/media-device.h>
 #include <media/media-entity.h>
@@ -150,6 +151,9 @@ struct parent_dev_ops {
 	void __iomem *(*get_base_address)(struct camss *camss, int id);
 };
 
+int camss_parse_endpoint_node(struct device *dev,
+			      struct fwnode_handle *ep,
+			      struct camss_async_subdev *csd);
 void camss_add_clock_margin(u64 *rate);
 int camss_enable_clocks(int nclocks, struct camss_clock *clock,
 			struct device *dev);
-- 
2.49.0


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

* [PATCH 07/10] media: qcom: camss: csiphy: probe any present children CSIPHY subdevices
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
                   ` (5 preceding siblings ...)
  2025-06-12  1:15 ` [PATCH 06/10] media: qcom: camss: export camss_parse_endpoint_node() to csiphy Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

Add CSIPHY driver routines to catch any possible csiphy subdevices
under CAMSS device tree node.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 .../media/platform/qcom/camss/camss-csiphy.c  | 248 ++++++++++++++++++
 .../media/platform/qcom/camss/camss-csiphy.h  |   3 +
 drivers/media/platform/qcom/camss/camss.c     |   2 +
 3 files changed, 253 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index f561811b7617..3020f7d0f621 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -17,6 +17,7 @@
 #include <linux/pm_runtime.h>
 #include <media/media-entity.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
 #include "camss-csiphy.h"
@@ -24,6 +25,17 @@
 
 #define MSM_CSIPHY_NAME "msm_csiphy"
 
+struct csiphy_priv {
+	struct device *dev;
+	unsigned int id;
+	struct camss *camss;
+	struct csiphy_device *csiphy;
+
+	struct v4l2_async_notifier notifier;
+
+	bool combo_mode;
+};
+
 static const struct csiphy_format_info formats_8x16[] = {
 	{ MEDIA_BUS_FMT_UYVY8_1X16, 8 },
 	{ MEDIA_BUS_FMT_VYUY8_1X16, 8 },
@@ -836,3 +848,239 @@ void msm_csiphy_unregister_entity(struct csiphy_device *csiphy)
 	v4l2_device_unregister_subdev(&csiphy->subdev);
 	media_entity_cleanup(&csiphy->subdev.entity);
 }
+
+static int csiphy_notify_bound(struct v4l2_async_notifier *notifier,
+			       struct v4l2_subdev *subdev,
+			       struct v4l2_async_connection *asd)
+{
+	struct csiphy_priv *csiphy_priv = container_of(notifier,
+						struct csiphy_priv, notifier);
+	struct camss_async_subdev *csd = container_of(asd,
+						struct camss_async_subdev, asd);
+	struct csiphy_device *csiphy = csiphy_priv->csiphy;
+	struct media_entity *sensor = &subdev->entity;
+	unsigned int i;
+
+	/* Keep parsed media interface data, but set the correct port id */
+	csiphy->id = csiphy_priv->id;
+	csiphy->cfg.csi2 = &csd->interface.csi2;
+
+	for (i = 0; i < sensor->num_pads; i++)
+		if (sensor->pads[i].flags & MEDIA_PAD_FL_SOURCE)
+			break;
+
+	if (i == sensor->num_pads) {
+		dev_err(csiphy_priv->dev, "No source pad in external entity\n");
+		return -EINVAL;
+	}
+
+	return media_create_pad_link(sensor, i, &csiphy->subdev.entity,
+				MSM_CSIPHY_PAD_SINK,
+				MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+}
+
+static int csiphy_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct csiphy_priv *csiphy = container_of(notifier, struct csiphy_priv,
+						  notifier);
+	struct camss *camss = csiphy->camss;
+
+	return v4l2_device_register_subdev_nodes(&camss->v4l2_dev);
+}
+
+static const struct v4l2_async_notifier_operations csiphy_notify_ops = {
+	.bound = csiphy_notify_bound,
+	.complete = csiphy_notify_complete,
+};
+
+static int msm_csiphy_parse_ports(struct csiphy_priv *csiphy)
+{
+	struct device *dev = csiphy->dev;
+	struct fwnode_handle *fwnode = dev_fwnode(dev), *ep;
+	unsigned int num_endpoints = fwnode_graph_get_endpoint_count(fwnode,
+						FWNODE_GRAPH_DEVICE_DISABLED);
+	int ret;
+
+	switch (num_endpoints) {
+	case 0:
+		return 0;
+	case 1:
+		break;
+	case 2:
+		csiphy->combo_mode = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	v4l2_async_nf_init(&csiphy->notifier, &csiphy->camss->v4l2_dev);
+	csiphy->notifier.ops = &csiphy_notify_ops;
+
+	fwnode_graph_for_each_endpoint(fwnode, ep) {
+		struct camss_async_subdev *csd;
+
+		csd = v4l2_async_nf_add_fwnode_remote(&csiphy->notifier, ep,
+						struct camss_async_subdev);
+		if (IS_ERR(csd)) {
+			ret = PTR_ERR(csd);
+			goto err_remote;
+		}
+
+		ret = camss_parse_endpoint_node(dev, ep, csd);
+		if (ret < 0)
+			goto err_remote;
+	}
+
+	ret = v4l2_async_nf_register(&csiphy->notifier);
+	if (ret)
+		goto err_cleanup;
+
+	return 0;
+
+err_remote:
+	fwnode_handle_put(ep);
+err_cleanup:
+	v4l2_async_nf_cleanup(&csiphy->notifier);
+
+	return ret;
+}
+
+static int msm_csiphy_init(struct csiphy_priv *csiphy)
+{
+	struct camss *camss = csiphy->camss;
+	unsigned int i = csiphy->id, j;
+	int ret;
+
+	ret = msm_csiphy_subdev_init(camss, &camss->csiphy[i], i);
+	if (ret < 0) {
+		dev_err(csiphy->dev, "Failed to init csiphy%d sub-device: %d\n",
+			i, ret);
+		return ret;
+	}
+
+	ret = msm_csiphy_register_entity(&camss->csiphy[i], &camss->v4l2_dev);
+	if (ret < 0) {
+		dev_err(csiphy->dev, "Failed to register csiphy%d entity: %d\n",
+			i, ret);
+		return ret;
+	}
+
+	for (j = 0; j < camss->res->csid_num; j++) {
+		ret = media_create_pad_link(&camss->csiphy[i].subdev.entity,
+					    1 /* source */,
+					    &camss->csid[j].subdev.entity,
+					    0 /* sink */,
+					    0);
+		if (ret < 0) {
+			dev_err(csiphy->dev,
+				"Failed to link csiphy%d to csid: %d\n",
+				i, ret);
+			msm_csiphy_unregister_entity(&camss->csiphy[i]);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int msm_csiphy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *phy_node = dev_of_node(dev), *camss_node;
+	struct csiphy_priv *csiphy_priv;
+	struct of_phandle_iterator it;
+	struct camss *camss;
+	unsigned int i = 0;
+	int ret;
+
+	/* Bail out if camss device driver has not yet been registered */
+	camss = dev_get_drvdata(dev->parent);
+	if (!camss || !camss->v4l2_dev.dev)
+		return -EPROBE_DEFER;
+
+	camss_node = of_get_parent(phy_node);
+	if (!camss_node)
+		return -ENODEV;
+
+	of_for_each_phandle(&it, ret, camss_node, "phys", "#phy-cells", -1) {
+		if (it.node != phy_node) {
+			i++;
+			continue;
+		}
+
+		if (!of_node_name_eq(it.node, "phy") ||
+		    !of_device_is_available(it.node))
+			ret = -ENODEV;
+
+		of_node_put(it.node);
+		break;
+	}
+	of_node_put(camss_node);
+
+	if (ret)
+		return ret;
+
+	if (i >= camss->res->csiphy_num)
+		return -EINVAL;
+
+	csiphy_priv = devm_kzalloc(dev, sizeof(*csiphy_priv), GFP_KERNEL);
+	if (!csiphy_priv)
+		return -ENOMEM;
+
+	csiphy_priv->dev = dev;
+	csiphy_priv->camss = camss;
+	csiphy_priv->id = i;
+	csiphy_priv->csiphy = &camss->csiphy[i];
+
+	ret = msm_csiphy_init(csiphy_priv);
+	if (ret < 0)
+		goto err_parse;
+
+	ret = msm_csiphy_parse_ports(csiphy_priv);
+	if (ret < 0)
+		goto err_cleanup;
+
+	dev_set_drvdata(dev, csiphy_priv);
+
+	return 0;
+
+err_cleanup:
+	msm_csiphy_unregister_entity(csiphy_priv->csiphy);
+err_parse:
+	v4l2_async_nf_unregister(&csiphy_priv->notifier);
+	v4l2_async_nf_cleanup(&csiphy_priv->notifier);
+
+	return ret;
+}
+
+static void msm_csiphy_remove(struct platform_device *pdev)
+{
+	struct csiphy_priv *csiphy_priv = dev_get_drvdata(&pdev->dev);
+
+	msm_csiphy_unregister_entity(csiphy_priv->csiphy);
+
+	v4l2_async_nf_unregister(&csiphy_priv->notifier);
+	v4l2_async_nf_cleanup(&csiphy_priv->notifier);
+}
+
+static const struct of_device_id csiphy_dt_match[] = {
+	{ .compatible = "qcom,csiphy", },
+	{}
+};
+
+static struct platform_driver csiphy_platform_driver = {
+	.probe	= msm_csiphy_probe,
+	.remove	= msm_csiphy_remove,
+	.driver	= {
+		.name		= "qcom-csiphy",
+		.of_match_table	= csiphy_dt_match,
+	},
+};
+
+void __init msm_csiphy_driver_register(void) {
+	platform_driver_register(&csiphy_platform_driver);
+}
+
+void __exit msm_csiphy_driver_unregister(void) {
+	platform_driver_unregister(&csiphy_platform_driver);
+}
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index f092b7ff2f26..b984aa745c78 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -120,6 +120,9 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
 
 void msm_csiphy_unregister_entity(struct csiphy_device *csiphy);
 
+void msm_csiphy_driver_register(void);
+void msm_csiphy_driver_unregister(void);
+
 extern const struct csiphy_formats csiphy_formats_8x16;
 extern const struct csiphy_formats csiphy_formats_8x96;
 extern const struct csiphy_formats csiphy_formats_sdm845;
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 40bb20bbe8b4..57a522fcb8c0 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -3955,6 +3955,7 @@ static struct platform_driver qcom_camss_driver = {
 
 static int __init qcom_camss_init(void)
 {
+	msm_csiphy_driver_register();
 	return platform_driver_register(&qcom_camss_driver);
 }
 module_init(qcom_camss_init);
@@ -3962,6 +3963,7 @@ module_init(qcom_camss_init);
 static void __exit qcom_camss_exit(void)
 {
 	platform_driver_unregister(&qcom_camss_driver);
+	msm_csiphy_driver_unregister();
 }
 module_exit(qcom_camss_exit);
 
-- 
2.49.0


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

* [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
                   ` (6 preceding siblings ...)
  2025-06-12  1:15 ` [PATCH 07/10] media: qcom: camss: csiphy: probe any present children CSIPHY subdevices Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  7:25   ` Krzysztof Kozlowski
                     ` (2 more replies)
  2025-06-12  1:15 ` [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices Vladimir Zapolskiy
  2025-06-12  1:15 ` [PATCH 10/10] [RFT] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: switch to new CSIPHY scheme Vladimir Zapolskiy
  9 siblings, 3 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
RFC verion of the change:
* https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/

Changes from RFC to v1:
* moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
* added 'clock-names' property,
* removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
* the above implies a removal of SM8250 specific supplies.

 .../bindings/media/qcom,csiphy.yaml           | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,csiphy.yaml

diff --git a/Documentation/devicetree/bindings/media/qcom,csiphy.yaml b/Documentation/devicetree/bindings/media/qcom,csiphy.yaml
new file mode 100644
index 000000000000..699824e7c44d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,csiphy.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,csiphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm CSI PHY
+
+maintainers:
+  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
+
+description:
+  Qualcomm SoCs equipped with a number of MIPI CSI PHY IPs, which
+  supports D-PHY or C-PHY interfaces to camera sensors.
+
+properties:
+  compatible:
+    enum:
+      - qcom,csiphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: csiphy
+      - const: csiphy_timer
+
+  interrupts:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    description: CAMSS CSIPHY input port
+
+    patternProperties:
+      "^endpoint@[0-1]$":
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            minItems: 1
+            maxItems: 4
+
+          bus-type:
+            enum:
+              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
+              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
+
+        required:
+          - data-lanes
+
+    oneOf:
+      - required:
+          - endpoint
+      - required:
+          - endpoint@0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - "#phy-cells"
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,camcc-sm8250.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    phy@ac6e000 {
+      compatible = "qcom,csiphy";
+      reg = <0x0ac6e000 0x1000>;
+      clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
+               <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
+      clock-names = "csiphy", "csiphy_timer";
+      interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
+      #phy-cells = <0>;
+
+      port {
+        csiphy_in: endpoint {
+          data-lanes = <1 2 3 4>;
+          remote-endpoint = <&sensor_out>;
+        };
+      };
+    };
-- 
2.49.0


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

* [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
                   ` (7 preceding siblings ...)
  2025-06-12  1:15 ` [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  2025-06-12  7:43   ` Krzysztof Kozlowski
                     ` (2 more replies)
  2025-06-12  1:15 ` [PATCH 10/10] [RFT] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: switch to new CSIPHY scheme Vladimir Zapolskiy
  9 siblings, 3 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

Following the new device tree bindings for CAMSS IPs introduce csiphy2
device tree node under SM8250 CAMSS, which allows to perform camera
tests of the model on an RB5 board with an attached vision mezzanine.

Note that the optional 'phys' property is deliberately not added.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
For testing only, do not merge.

 arch/arm64/boot/dts/qcom/sm8250.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index f0d18fd37aaf..401a32679580 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -4613,6 +4613,10 @@ camss: camss@ac6a000 {
 					     "cam_sf_0_mnoc",
 					     "cam_sf_icp_mnoc";
 
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -4641,6 +4645,16 @@ port@5 {
 					reg = <5>;
 				};
 			};
+
+			csiphy2: phy@ac6e000 {
+				compatible = "qcom,csiphy";
+				reg = <0 0x0ac6e000 0 0x1000>;
+				clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
+					 <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
+				clock-names = "csiphy", "csiphy_timer";
+				interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
+				#phy-cells = <0>;
+			};
 		};
 
 		camcc: clock-controller@ad00000 {
-- 
2.49.0


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

* [PATCH 10/10] [RFT] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: switch to new CSIPHY scheme
  2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
                   ` (8 preceding siblings ...)
  2025-06-12  1:15 ` [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices Vladimir Zapolskiy
@ 2025-06-12  1:15 ` Vladimir Zapolskiy
  9 siblings, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  1:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

The change provides a working example of using a new scheme of describing
CSIPHY devices under CAMSS.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
For testing only, do not merge.

 .../dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
index 5fe331923dd3..939eec7a039f 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
@@ -14,18 +14,20 @@ &camcc {
 };
 
 &camss {
+	status = "okay";
 	vdda-phy-supply = <&vreg_l5a_0p88>;
 	vdda-pll-supply = <&vreg_l9a_1p2>;
+
+	phys = <0>, <0>, <&csiphy2>, <0>, <0>, <0>;
+};
+
+&csiphy2 {
 	status = "okay";
 
-	ports {
-		/* The port index denotes CSIPHY id i.e. csiphy2 */
-		port@2 {
-			csiphy2_ep: endpoint {
-				clock-lanes = <7>;
-				data-lanes = <0 1 2 3>;
-				remote-endpoint = <&imx577_ep>;
-			};
+	port {
+		csiphy2_ep: endpoint {
+			data-lanes = <0 1 2 3>;
+			remote-endpoint = <&imx577_ep>;
 		};
 	};
 };
-- 
2.49.0


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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  1:15 ` [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
@ 2025-06-12  7:25   ` Krzysztof Kozlowski
  2025-06-12  7:38   ` Krzysztof Kozlowski
  2025-08-11 10:53   ` Dmitry Baryshkov
  2 siblings, 0 replies; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-12  7:25 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> RFC verion of the change:
> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
> 
> Changes from RFC to v1:

That was a v1. We ALWAYS start from 1, not -1, -3 or whatever RFC is. Look:

b4 diff '<20250612011531.2923701-9-vladimir.zapolskiy@linaro.org>'
Grabbing thread from
lore.kernel.org/all/20250612011531.2923701-9-vladimir.zapolskiy@linaro.org/t.mbox.gz
Breaking thread to remove parents of
20250612011531.2923701-1-vladimir.zapolskiy@linaro.org
---
Analyzing 11 messages in the thread
Could not find lower series to compare against.


Best regards,
Krzysztof

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

* Re: [PATCH 01/10] media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put()
  2025-06-12  1:15 ` [PATCH 01/10] media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put() Vladimir Zapolskiy
@ 2025-06-12  7:31   ` Bryan O'Donoghue
  0 siblings, 0 replies; 42+ messages in thread
From: Bryan O'Donoghue @ 2025-06-12  7:31 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 02:15, Vladimir Zapolskiy wrote:
> Two intended to be helpers camss_vfe_get()/camss_vfe_put() got their
> declarations in commit b1e6eef535df ("media: qcom: camss: Decouple VFE
> from CSID"), but the correspondent functions haven't beed even added.
> 
> Remove the unused declarations.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>   drivers/media/platform/qcom/camss/camss.h | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 1d0f83e4a2c9..99831846ebb5 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -160,8 +160,6 @@ s64 camss_get_link_freq(struct media_entity *entity, unsigned int bpp,
>   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,
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

* Re: [PATCH 02/10] media: qcom: camss: remove subdev resource argument from msm_csiphy_subdev_init()
  2025-06-12  1:15 ` [PATCH 02/10] media: qcom: camss: remove subdev resource argument from msm_csiphy_subdev_init() Vladimir Zapolskiy
@ 2025-06-12  7:37   ` Bryan O'Donoghue
  0 siblings, 0 replies; 42+ messages in thread
From: Bryan O'Donoghue @ 2025-06-12  7:37 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 02:15, Vladimir Zapolskiy wrote:
> The removed argument is directly and unambiguously derived from other
> ones.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>   drivers/media/platform/qcom/camss/camss-csiphy.c | 6 +++---
>   drivers/media/platform/qcom/camss/camss-csiphy.h | 5 +----
>   drivers/media/platform/qcom/camss/camss.c        | 1 -
>   3 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index 2de97f58f9ae..1ba3fc2e33ac 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -569,16 +569,16 @@ static bool csiphy_match_clock_name(const char *clock_name, const char *format,
>   
>   /*
>    * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
> + * @camss: CAMSS device
>    * @csiphy: CSIPHY device
> - * @res: CSIPHY module resources table
>    * @id: CSIPHY module id
>    *
>    * Return 0 on success or a negative error code otherwise
>    */
>   int msm_csiphy_subdev_init(struct camss *camss,
> -			   struct csiphy_device *csiphy,
> -			   const struct camss_subdev_resources *res, u8 id)
> +			   struct csiphy_device *csiphy, u8 id)
>   {
> +	const struct camss_subdev_resources *res = &camss->res->csiphy_res[id];
>   	struct device *dev = camss->dev;
>   	struct platform_device *pdev = to_platform_device(dev);
>   	int i, j;
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
> index 895f80003c44..d82dafd1d270 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
> @@ -113,11 +113,8 @@ struct csiphy_device {
>   	struct csiphy_device_regs *regs;
>   };
>   
> -struct camss_subdev_resources;
> -
>   int msm_csiphy_subdev_init(struct camss *camss,
> -			   struct csiphy_device *csiphy,
> -			   const struct camss_subdev_resources *res, u8 id);
> +			   struct csiphy_device *csiphy, u8 id);
>   
>   int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>   			       struct v4l2_device *v4l2_dev);
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 0d05f52a6e92..695f113472a5 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -3074,7 +3074,6 @@ static int camss_init_subdevices(struct camss *camss)
>   
>   	for (i = 0; i < camss->res->csiphy_num; i++) {
>   		ret = msm_csiphy_subdev_init(camss, &camss->csiphy[i],
> -					     &res->csiphy_res[i],
>   					     res->csiphy_res[i].csiphy.id);
>   		if (ret < 0) {
>   			dev_err(camss->dev,
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  1:15 ` [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
  2025-06-12  7:25   ` Krzysztof Kozlowski
@ 2025-06-12  7:38   ` Krzysztof Kozlowski
  2025-06-12  7:39     ` Krzysztof Kozlowski
  2025-06-12 17:13     ` Vladimir Zapolskiy
  2025-08-11 10:53   ` Dmitry Baryshkov
  2 siblings, 2 replies; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-12  7:38 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> RFC verion of the change:
> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
> 
> Changes from RFC to v1:
> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
> * added 'clock-names' property,
> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
> * the above implies a removal of SM8250 specific supplies.
> 
>  .../bindings/media/qcom,csiphy.yaml           | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/qcom,csiphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,csiphy.yaml b/Documentation/devicetree/bindings/media/qcom,csiphy.yaml
> new file mode 100644
> index 000000000000..699824e7c44d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,csiphy.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,csiphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm CSI PHY
> +
> +maintainers:
> +  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> +
> +description:
> +  Qualcomm SoCs equipped with a number of MIPI CSI PHY IPs, which
> +  supports D-PHY or C-PHY interfaces to camera sensors.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,csiphy

Instead soc specific compatible and adjust filename as well.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: csiphy
> +      - const: csiphy_timer

Drop csiphy from both, redundant. And this points to the first clock
name not having any useful name. Name equal to device name is not useful.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    description: CAMSS CSIPHY input port

Missing additionalProperties: false

> +
> +    patternProperties:
> +      "^endpoint@[0-1]$":
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +          bus-type:
> +            enum:
> +              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> +              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> +        required:
> +          - data-lanes
> +
> +    oneOf:
> +      - required:
> +          - endpoint

Your schema does not allow this.

> +      - required:
> +          - endpoint@0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +  - "#phy-cells"

port is not required? I expect it to be - how this can work without
wiring to the sensor?

You also miss to update the schema using it as a child - camss or
whatever is there.



Best regards,
Krzysztof

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  7:38   ` Krzysztof Kozlowski
@ 2025-06-12  7:39     ` Krzysztof Kozlowski
  2025-06-12  7:57       ` Vladimir Zapolskiy
  2025-06-12 17:13     ` Vladimir Zapolskiy
  1 sibling, 1 reply; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-12  7:39 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>> RFC verion of the change:
>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>
>> Changes from RFC to v1:
>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>> * added 'clock-names' property,
>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,


Now I noticed this... weird change and clearly a no-go.

Device binding cannot be generic, so it is not good enough for now.
Please write specific bindings for specific hardware.


Best regards,
Krzysztof

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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-06-12  1:15 ` [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices Vladimir Zapolskiy
@ 2025-06-12  7:43   ` Krzysztof Kozlowski
  2025-06-12 16:25     ` Konrad Dybcio
  2025-06-12 17:03     ` Vladimir Zapolskiy
  2025-06-23  9:31   ` Neil Armstrong
  2025-08-07 12:37   ` Bryan O'Donoghue
  2 siblings, 2 replies; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-12  7:43 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
> Following the new device tree bindings for CAMSS IPs introduce csiphy2
> device tree node under SM8250 CAMSS, which allows to perform camera
> tests of the model on an RB5 board with an attached vision mezzanine.

How the binding allows to perform camera tests? So camera was not
working here at all? Then this is a fix, no?


> 
> Note that the optional 'phys' property is deliberately not added.

Why? Your commit msg must explain that.

> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> For testing only, do not merge.
> 
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index f0d18fd37aaf..401a32679580 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4613,6 +4613,10 @@ camss: camss@ac6a000 {
>  					     "cam_sf_0_mnoc",
>  					     "cam_sf_icp_mnoc";
>  
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> @@ -4641,6 +4645,16 @@ port@5 {
>  					reg = <5>;
>  				};
>  			};
> +
> +			csiphy2: phy@ac6e000 {

This will fail checks. You can run them, regardless of "RFT" status.

> +				compatible = "qcom,csiphy";
> +				reg = <0 0x0ac6e000 0 0x1000>;
> +				clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
> +					 <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
> +				clock-names = "csiphy", "csiphy_timer";
> +				interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
> +				#phy-cells = <0>;

This is also duplicating existing ports thus you have a mixed MMIO and
non-MMIO children which is also issue to fix.

> +			};
>  		};
>  
>  		camcc: clock-controller@ad00000 {


Best regards,
Krzysztof

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  7:39     ` Krzysztof Kozlowski
@ 2025-06-12  7:57       ` Vladimir Zapolskiy
  2025-06-12 11:02         ` Krzysztof Kozlowski
  2025-06-12 16:17         ` Konrad Dybcio
  0 siblings, 2 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12  7:57 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 10:39, Krzysztof Kozlowski wrote:
> On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> ---
>>> RFC verion of the change:
>>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>>
>>> Changes from RFC to v1:
>>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>>> * added 'clock-names' property,
>>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
> 
> 
> Now I noticed this... weird change and clearly a no-go.
> 
> Device binding cannot be generic, so it is not good enough for now.
> Please write specific bindings for specific hardware.
> 

Can I add platform specific changes on top of the displayed generic one
like in Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
etc?

The generic compatible is sufficienlty good for adding the enhanced
CSIPHY support to any currently present in the upstream platform CAMSS.

Obviously I can rename it to something SoC-specific, but then a question
arises, if a selected platform has to be a totally new one in the upstream,
or it could be among any of platforms with a ready CAMSS, and a backward
compatibility is preserved by these series and the new CSIPHY dt bindings.

--
Best wishes,
Vladimir

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

* Re: [PATCH 03/10] media: qcom: camss: csiphy: simplify arguments of lanes_enable and lanes_disable
  2025-06-12  1:15 ` [PATCH 03/10] media: qcom: camss: csiphy: simplify arguments of lanes_enable and lanes_disable Vladimir Zapolskiy
@ 2025-06-12  8:02   ` Bryan O'Donoghue
  0 siblings, 0 replies; 42+ messages in thread
From: Bryan O'Donoghue @ 2025-06-12  8:02 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 02:15, Vladimir Zapolskiy wrote:
> In some of .lanes_enable and .lanes_disable functions the second argument
> of csiphy_config type is either unused or it can be derived from the
> main function argument, this lets to remove it from the list of arguments.
> 
> Apart of being the simplification the change is needed for further updates
> to CSIPHY part of the CAMSS driver to get CSIPHY combo mode feature and
> a related to it management of non-statically assigned CSIPHY media pads.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>   .../media/platform/qcom/camss/camss-csiphy-2ph-1-0.c  | 10 ++++------
>   .../media/platform/qcom/camss/camss-csiphy-3ph-1-0.c  | 11 ++++-------
>   drivers/media/platform/qcom/camss/camss-csiphy.c      |  4 ++--
>   drivers/media/platform/qcom/camss/camss-csiphy.h      |  4 +---
>   4 files changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c
> index 9d67e7fa6366..d9735f61fffc 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-2ph-1-0.c
> @@ -95,10 +95,9 @@ static u8 csiphy_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
>   }
>   
>   static void csiphy_lanes_enable(struct csiphy_device *csiphy,
> -				struct csiphy_config *cfg,
>   				s64 link_freq, u8 lane_mask)
>   {
> -	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
> +	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
>   	u8 settle_cnt;
>   	u8 val, l = 0;
>   	int i = 0;
> @@ -114,7 +113,7 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>   	val |= lane_mask << 1;
>   	writel_relaxed(val, csiphy->base + CAMSS_CSI_PHY_GLBL_PWR_CFG);
>   
> -	val = cfg->combo_mode << 4;
> +	val = csiphy->cfg.combo_mode << 4;
>   	writel_relaxed(val, csiphy->base + CAMSS_CSI_PHY_GLBL_RESET);
>   
>   	for (i = 0; i <= c->num_data; i++) {
> @@ -134,10 +133,9 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>   	}
>   }
>   
> -static void csiphy_lanes_disable(struct csiphy_device *csiphy,
> -				 struct csiphy_config *cfg)
> +static void csiphy_lanes_disable(struct csiphy_device *csiphy)
>   {
> -	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
> +	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
>   	u8 l = 0;
>   	int i = 0;
>   
> 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 f732a76de93e..69d95bfeb9d2 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
> @@ -638,10 +638,9 @@ static u8 csiphy_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
>   }
>   
>   static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,
> -				     struct csiphy_config *cfg,
>   				     u8 settle_cnt)
>   {
> -	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
> +	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
>   	int i, l = 0;
>   	u8 val;
>   
> @@ -758,10 +757,9 @@ static bool csiphy_is_gen2(u32 version)
>   }
>   
>   static void csiphy_lanes_enable(struct csiphy_device *csiphy,
> -				struct csiphy_config *cfg,
>   				s64 link_freq, u8 lane_mask)
>   {
> -	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
> +	struct csiphy_lanes_cfg *c = &csiphy->cfg.csi2->lane_cfg;
>   	struct csiphy_device_regs *regs = csiphy->regs;
>   	u8 settle_cnt;
>   	u8 val;
> @@ -791,7 +789,7 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>   	if (csiphy_is_gen2(csiphy->camss->res->version))
>   		csiphy_gen2_config_lanes(csiphy, settle_cnt);
>   	else
> -		csiphy_gen1_config_lanes(csiphy, cfg, settle_cnt);
> +		csiphy_gen1_config_lanes(csiphy, settle_cnt);
>   
>   	/* IRQ_MASK registers - disable all interrupts */
>   	for (i = 11; i < 22; i++) {
> @@ -800,8 +798,7 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>   	}
>   }
>   
> -static void csiphy_lanes_disable(struct csiphy_device *csiphy,
> -				 struct csiphy_config *cfg)
> +static void csiphy_lanes_disable(struct csiphy_device *csiphy)
>   {
>   	struct csiphy_device_regs *regs = csiphy->regs;
>   
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index 1ba3fc2e33ac..f561811b7617 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -295,7 +295,7 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>   		wmb();
>   	}
>   
> -	csiphy->res->hw_ops->lanes_enable(csiphy, cfg, link_freq, lane_mask);
> +	csiphy->res->hw_ops->lanes_enable(csiphy, link_freq, lane_mask);
>   
>   	return 0;
>   }
> @@ -308,7 +308,7 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>    */
>   static void csiphy_stream_off(struct csiphy_device *csiphy)
>   {
> -	csiphy->res->hw_ops->lanes_disable(csiphy, &csiphy->cfg);
> +	csiphy->res->hw_ops->lanes_disable(csiphy);
>   }
>   
>   
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
> index d82dafd1d270..3b73248f1364 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
> @@ -72,10 +72,8 @@ struct csiphy_hw_ops {
>   				struct device *dev);
>   	void (*reset)(struct csiphy_device *csiphy);
>   	void (*lanes_enable)(struct csiphy_device *csiphy,
> -			     struct csiphy_config *cfg,
>   			     s64 link_freq, u8 lane_mask);
> -	void (*lanes_disable)(struct csiphy_device *csiphy,
> -			      struct csiphy_config *cfg);
> +	void (*lanes_disable)(struct csiphy_device *csiphy);
>   	irqreturn_t (*isr)(int irq, void *dev);
>   	int (*init)(struct csiphy_device *csiphy);
>   };
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  7:57       ` Vladimir Zapolskiy
@ 2025-06-12 11:02         ` Krzysztof Kozlowski
  2025-06-12 11:27           ` Vladimir Zapolskiy
  2025-06-12 16:17         ` Konrad Dybcio
  1 sibling, 1 reply; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-12 11:02 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 09:57, Vladimir Zapolskiy wrote:
> On 6/12/25 10:39, Krzysztof Kozlowski wrote:
>> On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>> ---
>>>> RFC verion of the change:
>>>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>>>
>>>> Changes from RFC to v1:
>>>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>>>> * added 'clock-names' property,
>>>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
>>
>>
>> Now I noticed this... weird change and clearly a no-go.
>>
>> Device binding cannot be generic, so it is not good enough for now.
>> Please write specific bindings for specific hardware.
>>
> 
> Can I add platform specific changes on top of the displayed generic one
> like in Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> etc?
> 
> The generic compatible is sufficienlty good for adding the enhanced
> CSIPHY support to any currently present in the upstream platform CAMSS.
> 
> Obviously I can rename it to something SoC-specific, but then a question
> arises, if a selected platform has to be a totally new one in the upstream,
> or it could be among any of platforms with a ready CAMSS, and a backward
> compatibility is preserved by these series and the new CSIPHY dt bindings.

Just use a specific compatible for the actual hardware this is being
added for. I don't understand why this is different than all other work
upstream.

Best regards,
Krzysztof

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12 11:02         ` Krzysztof Kozlowski
@ 2025-06-12 11:27           ` Vladimir Zapolskiy
  2025-06-12 11:36             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12 11:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 14:02, Krzysztof Kozlowski wrote:
> On 12/06/2025 09:57, Vladimir Zapolskiy wrote:
>> On 6/12/25 10:39, Krzysztof Kozlowski wrote:
>>> On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>
>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>> ---
>>>>> RFC verion of the change:
>>>>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>>>>
>>>>> Changes from RFC to v1:
>>>>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>>>>> * added 'clock-names' property,
>>>>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
>>>
>>>
>>> Now I noticed this... weird change and clearly a no-go.
>>>
>>> Device binding cannot be generic, so it is not good enough for now.
>>> Please write specific bindings for specific hardware.
>>>
>>
>> Can I add platform specific changes on top of the displayed generic one
>> like in Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
>> etc?
>>
>> The generic compatible is sufficienlty good for adding the enhanced
>> CSIPHY support to any currently present in the upstream platform CAMSS.
>>
>> Obviously I can rename it to something SoC-specific, but then a question
>> arises, if a selected platform has to be a totally new one in the upstream,
>> or it could be among any of platforms with a ready CAMSS, and a backward
>> compatibility is preserved by these series and the new CSIPHY dt bindings.
> 
> Just use a specific compatible for the actual hardware this is being
> added for. I don't understand why this is different than all other work
> upstream.

There are very close examples in upstream, for instance that's a generic
value from Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:

properties:
   compatible:
     enum:
       - qcom,dsi-phy-10nm
       - qcom,dsi-phy-10nm-8998

To save time reviewing the next version of the same change, will you
accept a list of acceptable compatible properties like this one?

properties:
   compatible:
     enum:
       - qcom,csiphy
       - qcom,sm8250-csiphy

--
Best wishes,
Vladimir

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12 11:27           ` Vladimir Zapolskiy
@ 2025-06-12 11:36             ` Krzysztof Kozlowski
  0 siblings, 0 replies; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-12 11:36 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 13:27, Vladimir Zapolskiy wrote:
> On 6/12/25 14:02, Krzysztof Kozlowski wrote:
>> On 12/06/2025 09:57, Vladimir Zapolskiy wrote:
>>> On 6/12/25 10:39, Krzysztof Kozlowski wrote:
>>>> On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
>>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>>
>>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>>> ---
>>>>>> RFC verion of the change:
>>>>>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>>>>>
>>>>>> Changes from RFC to v1:
>>>>>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>>>>>> * added 'clock-names' property,
>>>>>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
>>>>
>>>>
>>>> Now I noticed this... weird change and clearly a no-go.
>>>>
>>>> Device binding cannot be generic, so it is not good enough for now.
>>>> Please write specific bindings for specific hardware.
>>>>
>>>
>>> Can I add platform specific changes on top of the displayed generic one
>>> like in Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
>>> etc?
>>>
>>> The generic compatible is sufficienlty good for adding the enhanced
>>> CSIPHY support to any currently present in the upstream platform CAMSS.
>>>
>>> Obviously I can rename it to something SoC-specific, but then a question
>>> arises, if a selected platform has to be a totally new one in the upstream,
>>> or it could be among any of platforms with a ready CAMSS, and a backward
>>> compatibility is preserved by these series and the new CSIPHY dt bindings.
>>
>> Just use a specific compatible for the actual hardware this is being
>> added for. I don't understand why this is different than all other work
>> upstream.
> 
> There are very close examples in upstream, for instance that's a generic
> value from Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:
> 
> properties:
>    compatible:
>      enum:
>        - qcom,dsi-phy-10nm
>        - qcom,dsi-phy-10nm-8998

That's ancient now style. Don't use something from 10 years ago as example.

> 
> To save time reviewing the next version of the same change, will you
> accept a list of acceptable compatible properties like this one?
> 
> properties:
>    compatible:
>      enum:
>        - qcom,csiphy

No. You cannot have generic compatible. We keep repeating this all the
time, so this is nothing new.

>        - qcom,sm8250-csiphy
> 

Best regards,
Krzysztof

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  7:57       ` Vladimir Zapolskiy
  2025-06-12 11:02         ` Krzysztof Kozlowski
@ 2025-06-12 16:17         ` Konrad Dybcio
  2025-06-12 16:44           ` Vladimir Zapolskiy
  1 sibling, 1 reply; 42+ messages in thread
From: Konrad Dybcio @ 2025-06-12 16:17 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 9:57 AM, Vladimir Zapolskiy wrote:
> On 6/12/25 10:39, Krzysztof Kozlowski wrote:
>> On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>> ---
>>>> RFC verion of the change:
>>>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>>>
>>>> Changes from RFC to v1:
>>>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>>>> * added 'clock-names' property,
>>>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
>>
>>
>> Now I noticed this... weird change and clearly a no-go.
>>
>> Device binding cannot be generic, so it is not good enough for now.
>> Please write specific bindings for specific hardware.
>>
> 
> Can I add platform specific changes on top of the displayed generic one
> like in Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> etc?
> 
> The generic compatible is sufficienlty good for adding the enhanced
> CSIPHY support to any currently present in the upstream platform CAMSS.
> 
> Obviously I can rename it to something SoC-specific, but then a question
> arises, if a selected platform has to be a totally new one in the upstream,
> or it could be among any of platforms with a ready CAMSS, and a backward
> compatibility is preserved by these series and the new CSIPHY dt bindings.

A YAML file hosting common properties will probably be very welcome, but
the compatibles must be specific to avoid having to redo this dance in
a couple years..

Then, the camera ip is well-versioned, so you can use that as the 'specific'
part. It'll also make it easier to resolve the unlikely case of a SoC using
a mix of different PHY versions.

Konrad

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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-06-12  7:43   ` Krzysztof Kozlowski
@ 2025-06-12 16:25     ` Konrad Dybcio
  2025-06-12 17:03     ` Vladimir Zapolskiy
  1 sibling, 0 replies; 42+ messages in thread
From: Konrad Dybcio @ 2025-06-12 16:25 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Vladimir Zapolskiy, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 9:43 AM, Krzysztof Kozlowski wrote:
> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>> Following the new device tree bindings for CAMSS IPs introduce csiphy2
>> device tree node under SM8250 CAMSS, which allows to perform camera
>> tests of the model on an RB5 board with an attached vision mezzanine.

[...]

>> +				compatible = "qcom,csiphy";
>> +				reg = <0 0x0ac6e000 0 0x1000>;
>> +				clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
>> +					 <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
>> +				clock-names = "csiphy", "csiphy_timer";
>> +				interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
>> +				#phy-cells = <0>;
> 
> This is also duplicating existing ports thus you have a mixed MMIO and
> non-MMIO children which is also issue to fix.

Does the CSIPHY work without the camera ahb clk/TOP GDSC enabled?
(as far as register access and doing things that don't depend
on input from other components)

I think moving it outside makes more sense both in a "breaking up
the monolith" way and in case the answer to the question is
positive, in representing it as an individual device.

FWIW I'm generally in favor of what downstream does with regards
to camss - *every* instance of every device has its own node, as
it represents a physical subblock and has its individual clocks
etc.

I'm genuinely (not necessarily positively) surprised that we
managed to get this far with camss being a single big blob..

Konrad

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12 16:17         ` Konrad Dybcio
@ 2025-06-12 16:44           ` Vladimir Zapolskiy
  2025-06-14 19:29             ` Konrad Dybcio
  0 siblings, 1 reply; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12 16:44 UTC (permalink / raw)
  To: Konrad Dybcio, Krzysztof Kozlowski, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 19:17, Konrad Dybcio wrote:
> On 6/12/25 9:57 AM, Vladimir Zapolskiy wrote:
>> On 6/12/25 10:39, Krzysztof Kozlowski wrote:
>>> On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>
>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>> ---
>>>>> RFC verion of the change:
>>>>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>>>>
>>>>> Changes from RFC to v1:
>>>>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>>>>> * added 'clock-names' property,
>>>>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
>>>
>>>
>>> Now I noticed this... weird change and clearly a no-go.
>>>
>>> Device binding cannot be generic, so it is not good enough for now.
>>> Please write specific bindings for specific hardware.
>>>
>>
>> Can I add platform specific changes on top of the displayed generic one
>> like in Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
>> etc?
>>
>> The generic compatible is sufficienlty good for adding the enhanced
>> CSIPHY support to any currently present in the upstream platform CAMSS.
>>
>> Obviously I can rename it to something SoC-specific, but then a question
>> arises, if a selected platform has to be a totally new one in the upstream,
>> or it could be among any of platforms with a ready CAMSS, and a backward
>> compatibility is preserved by these series and the new CSIPHY dt bindings.
> 
> A YAML file hosting common properties will probably be very welcome, but
> the compatibles must be specific to avoid having to redo this dance in
> a couple years..

Right, that's a good way for sure, and I keep this option in my mind.

My concern is that it might be not a perfect fit particularly for CAMSS
CSIPHY IPs, because likely at least all currently supported in the upstream
CAMSS IPs will get one in one equal hardware descriptions, despite CSIPHY
IPs are obviously different. In other words I anticipate that there will
be just one platform prefixed YAML file with a long list of various platform
specific CSIPHYs, and therefore it's just one potential $ref user of this
hypothetical YAML file containing common device tree properties of CSIPHYs.

> Then, the camera ip is well-versioned, so you can use that as the 'specific'
> part. It'll also make it easier to resolve the unlikely case of a SoC using
> a mix of different PHY versions.
> 

Many thanks for input and reviews, regression test results of the given
CAMSS driver changes will be also very much appreciated, it may be helpful
for Bryan.

--
Best wishes,
Vladimir

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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-06-12  7:43   ` Krzysztof Kozlowski
  2025-06-12 16:25     ` Konrad Dybcio
@ 2025-06-12 17:03     ` Vladimir Zapolskiy
  1 sibling, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12 17:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 10:43, Krzysztof Kozlowski wrote:
> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>> Following the new device tree bindings for CAMSS IPs introduce csiphy2
>> device tree node under SM8250 CAMSS, which allows to perform camera
>> tests of the model on an RB5 board with an attached vision mezzanine.
> 
> How the binding allows to perform camera tests? So camera was not
> working here at all? Then this is a fix, no?

Here the assumed testing is a regression testing on the selected and
well supported legacy platform SM8250/RB5.

>>
>> Note that the optional 'phys' property is deliberately not added.
> 
> Why? Your commit msg must explain that.
> 

Sure, will add it, just giving a short note here, 'phys' property
provides a list of wanted/enabled phys, and this is board specific,
thus, and if it is desired to preserve backward ABI compatibility,
when this property is just not present, I believe it makes sense
to add the 'phys' property to board .dts files only, like it's
done in 10/10 patch of this series.

>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>> For testing only, do not merge.

This.

>>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> index f0d18fd37aaf..401a32679580 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> @@ -4613,6 +4613,10 @@ camss: camss@ac6a000 {
>>   					     "cam_sf_0_mnoc",
>>   					     "cam_sf_icp_mnoc";
>>   
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> +
>>   			ports {
>>   				#address-cells = <1>;
>>   				#size-cells = <0>;
>> @@ -4641,6 +4645,16 @@ port@5 {
>>   					reg = <5>;
>>   				};
>>   			};
>> +
>> +			csiphy2: phy@ac6e000 {
> 
> This will fail checks. You can run them, regardless of "RFT" status.

I ran "dt_binding_check" and "dtbs_check" with no errors reported,
something is screwed on my end, because "additionalProperties: false"
from qcom,sm8250-camss.yaml shall scream here... Or is it a child
device node is not a property?..

>> +				compatible = "qcom,csiphy";
>> +				reg = <0 0x0ac6e000 0 0x1000>;
>> +				clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
>> +					 <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
>> +				clock-names = "csiphy", "csiphy_timer";
>> +				interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
>> +				#phy-cells = <0>;
> 
> This is also duplicating existing ports thus you have a mixed MMIO and
> non-MMIO children which is also issue to fix.

That's correct, there are mixed MMIO and non-MMIO children above,
there might be children of just any one type of two.

It's a valuable review comment and I missed the flaw, thank you,
and to be honest so far I don't have a good idea how to overcome
the issue in an easy way...

>> +			};
>>   		};
>>   
>>   		camcc: clock-controller@ad00000 {
> 
> 

--
Best wishes,
Vladimir

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  7:38   ` Krzysztof Kozlowski
  2025-06-12  7:39     ` Krzysztof Kozlowski
@ 2025-06-12 17:13     ` Vladimir Zapolskiy
  2025-06-13  6:28       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-12 17:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 10:38, Krzysztof Kozlowski wrote:
> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---

<snip>

>> +
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    items:
>> +      - const: csiphy
>> +      - const: csiphy_timer
> 
> Drop csiphy from both, redundant. And this points to the first clock
> name not having any useful name. Name equal to device name is not useful.
> 

I got the rationale, but I have no idea how to correct it, since it's
literally the case, the first clock name on the list in 'csiphy'.

What could be an alternative name then?..

>> +
>> +    patternProperties:
>> +      "^endpoint@[0-1]$":
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes:
>> +            minItems: 1
>> +            maxItems: 4
>> +
>> +          bus-type:
>> +            enum:
>> +              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
>> +              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
>> +
>> +        required:
>> +          - data-lanes
>> +
>> +    oneOf:
>> +      - required:
>> +          - endpoint
> 
> Your schema does not allow this.
> 

That's the catched defect, thank you.

>> +      - required:
>> +          - endpoint@0
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - interrupts
>> +  - "#phy-cells"
> 
> port is not required? I expect it to be - how this can work without
> wiring to the sensor?

'port' is required here, you catched the defect.

> You also miss to update the schema using it as a child - camss or
> whatever is there.
> 

Agreed, some or even any currently present in the upstream CAMSS
schemas should be updated to account a new possible child device
node.

The pointed by you problem of MMIO/non-MMIO mix of children on CAMSS
side is still outstanding, apparently the usage of 'ports' or 'phys'
should be mutually exclusive.

Thank you so much for review!

--
Best wishes,
Vladimir

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12 17:13     ` Vladimir Zapolskiy
@ 2025-06-13  6:28       ` Krzysztof Kozlowski
  2025-06-14 19:31         ` Konrad Dybcio
  0 siblings, 1 reply; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-13  6:28 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 19:13, Vladimir Zapolskiy wrote:
> On 6/12/25 10:38, Krzysztof Kozlowski wrote:
>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> ---
> 
> <snip>
> 
>>> +
>>> +  clocks:
>>> +    maxItems: 2
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: csiphy
>>> +      - const: csiphy_timer
>>
>> Drop csiphy from both, redundant. And this points to the first clock
>> name not having any useful name. Name equal to device name is not useful.
>>
> 
> I got the rationale, but I have no idea how to correct it, since it's
> literally the case, the first clock name on the list in 'csiphy'.

What do you mean by "list"? You can point me also to internal
documentation if that helps.

> 
> What could be an alternative name then?..

The real clock input name, signal name. You can also drop the names.

Best regards,
Krzysztof

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12 16:44           ` Vladimir Zapolskiy
@ 2025-06-14 19:29             ` Konrad Dybcio
  0 siblings, 0 replies; 42+ messages in thread
From: Konrad Dybcio @ 2025-06-14 19:29 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Konrad Dybcio, Krzysztof Kozlowski,
	Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/12/25 6:44 PM, Vladimir Zapolskiy wrote:
> On 6/12/25 19:17, Konrad Dybcio wrote:
>> On 6/12/25 9:57 AM, Vladimir Zapolskiy wrote:
>>> On 6/12/25 10:39, Krzysztof Kozlowski wrote:
>>>> On 12/06/2025 09:38, Krzysztof Kozlowski wrote:
>>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>>
>>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>>> ---
>>>>>> RFC verion of the change:
>>>>>> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
>>>>>>
>>>>>> Changes from RFC to v1:
>>>>>> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
>>>>>> * added 'clock-names' property,
>>>>>> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
>>>>
>>>>
>>>> Now I noticed this... weird change and clearly a no-go.
>>>>
>>>> Device binding cannot be generic, so it is not good enough for now.
>>>> Please write specific bindings for specific hardware.
>>>>
>>>
>>> Can I add platform specific changes on top of the displayed generic one
>>> like in Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
>>> etc?
>>>
>>> The generic compatible is sufficienlty good for adding the enhanced
>>> CSIPHY support to any currently present in the upstream platform CAMSS.
>>>
>>> Obviously I can rename it to something SoC-specific, but then a question
>>> arises, if a selected platform has to be a totally new one in the upstream,
>>> or it could be among any of platforms with a ready CAMSS, and a backward
>>> compatibility is preserved by these series and the new CSIPHY dt bindings.
>>
>> A YAML file hosting common properties will probably be very welcome, but
>> the compatibles must be specific to avoid having to redo this dance in
>> a couple years..
> 
> Right, that's a good way for sure, and I keep this option in my mind.
> 
> My concern is that it might be not a perfect fit particularly for CAMSS
> CSIPHY IPs, because likely at least all currently supported in the upstream
> CAMSS IPs will get one in one equal hardware descriptions, despite CSIPHY
> IPs are obviously different. In other words I anticipate that there will
> be just one platform prefixed YAML file with a long list of various platform
> specific CSIPHYs, and therefore it's just one potential $ref user of this
> hypothetical YAML file containing common device tree properties of CSIPHYs.

One big YAML file may be okay too.. think:

compatible:
	enum:
		- qcom,csiphy-v1.0.0
		- qcom,csiphy-v1.2.0
		- qcom,csiphy-v2.0.0

clocks:
	// if oneOf then that many clocks, else that many

etc. etc.

> 
>> Then, the camera ip is well-versioned, so you can use that as the 'specific'
>> part. It'll also make it easier to resolve the unlikely case of a SoC using
>> a mix of different PHY versions.
>>
> 
> Many thanks for input and reviews, regression test results of the given
> CAMSS driver changes will be also very much appreciated, it may be helpful
> for Bryan.

I'm afraid I don't have any board which would be both unique and
with working camera..

Konrad

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-13  6:28       ` Krzysztof Kozlowski
@ 2025-06-14 19:31         ` Konrad Dybcio
  2025-06-17  6:32           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 42+ messages in thread
From: Konrad Dybcio @ 2025-06-14 19:31 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Vladimir Zapolskiy, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/13/25 8:28 AM, Krzysztof Kozlowski wrote:
> On 12/06/2025 19:13, Vladimir Zapolskiy wrote:
>> On 6/12/25 10:38, Krzysztof Kozlowski wrote:
>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>> ---
>>
>> <snip>
>>
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 2
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: csiphy
>>>> +      - const: csiphy_timer
>>>
>>> Drop csiphy from both, redundant. And this points to the first clock
>>> name not having any useful name. Name equal to device name is not useful.
>>>
>>
>> I got the rationale, but I have no idea how to correct it, since it's
>> literally the case, the first clock name on the list in 'csiphy'.
> 
> What do you mean by "list"? You can point me also to internal
> documentation if that helps.

So if you do:

"csiphy_timer" - "csiphy_" you're left with "timer" which makes sense

however, if you do:

"csiphy" - "csiphy_", you get "" and Vlad is wondering what to name it

> 
>>
>> What could be an alternative name then?..
> 
> The real clock input name, signal name. You can also drop the names.

I don't have the docs before my eyes right now, but I would not be
surprised if it's also called "csiphy" in there..

Konrad

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-14 19:31         ` Konrad Dybcio
@ 2025-06-17  6:32           ` Krzysztof Kozlowski
  2025-06-17  9:51             ` Vladimir Zapolskiy
  0 siblings, 1 reply; 42+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-17  6:32 UTC (permalink / raw)
  To: Konrad Dybcio, Vladimir Zapolskiy, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 14/06/2025 21:31, Konrad Dybcio wrote:
> On 6/13/25 8:28 AM, Krzysztof Kozlowski wrote:
>> On 12/06/2025 19:13, Vladimir Zapolskiy wrote:
>>> On 6/12/25 10:38, Krzysztof Kozlowski wrote:
>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>
>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>> ---
>>>
>>> <snip>
>>>
>>>>> +
>>>>> +  clocks:
>>>>> +    maxItems: 2
>>>>> +
>>>>> +  clock-names:
>>>>> +    items:
>>>>> +      - const: csiphy
>>>>> +      - const: csiphy_timer
>>>>
>>>> Drop csiphy from both, redundant. And this points to the first clock
>>>> name not having any useful name. Name equal to device name is not useful.
>>>>
>>>
>>> I got the rationale, but I have no idea how to correct it, since it's
>>> literally the case, the first clock name on the list in 'csiphy'.
>>
>> What do you mean by "list"? You can point me also to internal
>> documentation if that helps.
> 
> So if you do:
> 
> "csiphy_timer" - "csiphy_" you're left with "timer" which makes sense
> 
> however, if you do:
> 
> "csiphy" - "csiphy_", you get "" and Vlad is wondering what to name it

How is the signal named in HPG or diagram? It is possible it has a name
other than "csiphy"...

> 
>>
>>>
>>> What could be an alternative name then?..
>>
>> The real clock input name, signal name. You can also drop the names.
> 
> I don't have the docs before my eyes right now, but I would not be
> surprised if it's also called "csiphy" in there..

Let's check that first.

Best regards,
Krzysztof

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-17  6:32           ` Krzysztof Kozlowski
@ 2025-06-17  9:51             ` Vladimir Zapolskiy
  2025-06-17 19:20               ` Konrad Dybcio
  0 siblings, 1 reply; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-17  9:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Konrad Dybcio, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/17/25 09:32, Krzysztof Kozlowski wrote:
> On 14/06/2025 21:31, Konrad Dybcio wrote:
>> On 6/13/25 8:28 AM, Krzysztof Kozlowski wrote:
>>> On 12/06/2025 19:13, Vladimir Zapolskiy wrote:
>>>> On 6/12/25 10:38, Krzysztof Kozlowski wrote:
>>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>>
>>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>>> ---
>>>>
>>>> <snip>
>>>>
>>>>>> +
>>>>>> +  clocks:
>>>>>> +    maxItems: 2
>>>>>> +
>>>>>> +  clock-names:
>>>>>> +    items:
>>>>>> +      - const: csiphy
>>>>>> +      - const: csiphy_timer
>>>>>
>>>>> Drop csiphy from both, redundant. And this points to the first clock
>>>>> name not having any useful name. Name equal to device name is not useful.
>>>>>
>>>>
>>>> I got the rationale, but I have no idea how to correct it, since it's
>>>> literally the case, the first clock name on the list in 'csiphy'.
>>>
>>> What do you mean by "list"? You can point me also to internal
>>> documentation if that helps.
>>
>> So if you do:
>>
>> "csiphy_timer" - "csiphy_" you're left with "timer" which makes sense
>>
>> however, if you do:
>>
>> "csiphy" - "csiphy_", you get "" and Vlad is wondering what to name it
> 
> How is the signal named in HPG or diagram? It is possible it has a name
> other than "csiphy"...
> 
>>
>>>
>>>>
>>>> What could be an alternative name then?..
>>>
>>> The real clock input name, signal name. You can also drop the names.

That's the initial version of the change, there is a list of two clocks
as a value of 'clocks' property, but there is no 'clock-names' property.

But I suppose I can not keep it this way in v2 of the change.

>>
>> I don't have the docs before my eyes right now, but I would not be
>> surprised if it's also called "csiphy" in there..
> 
> Let's check that first.

Here only someone with the access to the specs can help, when I rely
on downstream code, and it says the right clock name here is 'csiphy'.

--
Best wishes,
Vladimir

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-17  9:51             ` Vladimir Zapolskiy
@ 2025-06-17 19:20               ` Konrad Dybcio
  2025-06-17 21:30                 ` Vladimir Zapolskiy
  0 siblings, 1 reply; 42+ messages in thread
From: Konrad Dybcio @ 2025-06-17 19:20 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/17/25 11:51 AM, Vladimir Zapolskiy wrote:
> On 6/17/25 09:32, Krzysztof Kozlowski wrote:
>> On 14/06/2025 21:31, Konrad Dybcio wrote:
>>> On 6/13/25 8:28 AM, Krzysztof Kozlowski wrote:
>>>> On 12/06/2025 19:13, Vladimir Zapolskiy wrote:
>>>>> On 6/12/25 10:38, Krzysztof Kozlowski wrote:
>>>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>>>
>>>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>>>> ---

[...]


>>> I don't have the docs before my eyes right now, but I would not be
>>> surprised if it's also called "csiphy" in there..
>>
>> Let's check that first.
> 
> Here only someone with the access to the specs can help, when I rely
> on downstream code, and it says the right clock name here is 'csiphy'.

Unfortunately, I can't find anything more descriptive than that.

Konrad

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-17 19:20               ` Konrad Dybcio
@ 2025-06-17 21:30                 ` Vladimir Zapolskiy
  0 siblings, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-17 21:30 UTC (permalink / raw)
  To: Konrad Dybcio, Krzysztof Kozlowski, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 6/17/25 22:20, Konrad Dybcio wrote:
> On 6/17/25 11:51 AM, Vladimir Zapolskiy wrote:
>> On 6/17/25 09:32, Krzysztof Kozlowski wrote:
>>> On 14/06/2025 21:31, Konrad Dybcio wrote:
>>>> On 6/13/25 8:28 AM, Krzysztof Kozlowski wrote:
>>>>> On 12/06/2025 19:13, Vladimir Zapolskiy wrote:
>>>>>> On 6/12/25 10:38, Krzysztof Kozlowski wrote:
>>>>>>> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>>>>>>>> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
>>>>>>>> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
>>>>>>>>
>>>>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>>>>> ---
> 
> [...]
> 
> 
>>>> I don't have the docs before my eyes right now, but I would not be
>>>> surprised if it's also called "csiphy" in there..
>>>
>>> Let's check that first.
>>
>> Here only someone with the access to the specs can help, when I rely
>> on downstream code, and it says the right clock name here is 'csiphy'.
> 
> Unfortunately, I can't find anything more descriptive than that.
> 

Thank you for checking it.

It might happen that a widely used clock name "core" serves the purpose,
so here I can introduce CSIPHY clocks "core" and "timer", if Krzysztof
also finds it acceptable.

--
Best wishes,
Vladimir

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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-06-12  1:15 ` [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices Vladimir Zapolskiy
  2025-06-12  7:43   ` Krzysztof Kozlowski
@ 2025-06-23  9:31   ` Neil Armstrong
  2025-06-23 13:06     ` Vladimir Zapolskiy
  2025-08-07 12:37   ` Bryan O'Donoghue
  2 siblings, 1 reply; 42+ messages in thread
From: Neil Armstrong @ 2025-06-23  9:31 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Vinod Koul, linux-arm-msm, linux-media, devicetree

On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
> Following the new device tree bindings for CAMSS IPs introduce csiphy2
> device tree node under SM8250 CAMSS, which allows to perform camera
> tests of the model on an RB5 board with an attached vision mezzanine.
> 
> Note that the optional 'phys' property is deliberately not added.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> For testing only, do not merge.
> 
>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index f0d18fd37aaf..401a32679580 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4613,6 +4613,10 @@ camss: camss@ac6a000 {
>   					     "cam_sf_0_mnoc",
>   					     "cam_sf_icp_mnoc";
>   
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +
>   			ports {
>   				#address-cells = <1>;
>   				#size-cells = <0>;
> @@ -4641,6 +4645,16 @@ port@5 {
>   					reg = <5>;
>   				};
>   			};
> +
> +			csiphy2: phy@ac6e000 {
> +				compatible = "qcom,csiphy";
> +				reg = <0 0x0ac6e000 0 0x1000>;
> +				clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
> +					 <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
> +				clock-names = "csiphy", "csiphy_timer";
> +				interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
> +				#phy-cells = <0>;
> +			};

I would've expected the CSI PHY nodes to be out of the camss node, why would you
keep them as subnodes since you would reference them via phys phandles ?

Neil

>   		};
>   
>   		camcc: clock-controller@ad00000 {


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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-06-23  9:31   ` Neil Armstrong
@ 2025-06-23 13:06     ` Vladimir Zapolskiy
  0 siblings, 0 replies; 42+ messages in thread
From: Vladimir Zapolskiy @ 2025-06-23 13:06 UTC (permalink / raw)
  To: Neil Armstrong, Krzysztof Kozlowski, Rob Herring, Bjorn Andersson,
	Konrad Dybcio, Bryan O'Donoghue
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Vinod Koul, linux-arm-msm, linux-media, devicetree

On 6/23/25 12:31, Neil Armstrong wrote:
> On 12/06/2025 03:15, Vladimir Zapolskiy wrote:
>> Following the new device tree bindings for CAMSS IPs introduce csiphy2
>> device tree node under SM8250 CAMSS, which allows to perform camera
>> tests of the model on an RB5 board with an attached vision mezzanine.
>>
>> Note that the optional 'phys' property is deliberately not added.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>> For testing only, do not merge.
>>
>>    arch/arm64/boot/dts/qcom/sm8250.dtsi | 14 ++++++++++++++
>>    1 file changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> index f0d18fd37aaf..401a32679580 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> @@ -4613,6 +4613,10 @@ camss: camss@ac6a000 {
>>    					     "cam_sf_0_mnoc",
>>    					     "cam_sf_icp_mnoc";
>>    
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> +
>>    			ports {
>>    				#address-cells = <1>;
>>    				#size-cells = <0>;
>> @@ -4641,6 +4645,16 @@ port@5 {
>>    					reg = <5>;
>>    				};
>>    			};
>> +
>> +			csiphy2: phy@ac6e000 {
>> +				compatible = "qcom,csiphy";
>> +				reg = <0 0x0ac6e000 0 0x1000>;
>> +				clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
>> +					 <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
>> +				clock-names = "csiphy", "csiphy_timer";
>> +				interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
>> +				#phy-cells = <0>;
>> +			};
> 
> I would've expected the CSI PHY nodes to be out of the camss node, why would you
> keep them as subnodes since you would reference them via phys phandles ?
> 

This is a good question, and it may require a deeper discussion.

Below are a few observations and comments supporting the idea of
describing CSIPHY IPs as subnodes of CAMSS device tree node.

1. Formally CSIPHY IPs are still parts of CAMSS controller, if the
CAMSS IP is considered as a multifunction device containing a number
of IP blocks, then it might be logically consistent to place new
children device tree nodes under its intermediate parent IP device
tree node rather than parent's parent device tree node.

2. Probably a consideration like the one above dictated a placement
of Qualcomm DSI PHY (and many other sub-IPs) device tree nodes under
a larger MDSS device tree node, here an attempt to repeat the same
layout is done.

3. If CSIPHY device tree nodes are completely detached from CAMSS
device tree node, then not just "phys" but also new endpoint to endpoint
links should be added between CSIPHYs and CSIDs provided by CAMSS like
it's dictated by the established scheme of media device connections,
however these particular endpoint links are non-fixed and configurable
in runtime.

The last point can be excluded only if there is a clear agreement that
a chain of media endpoint-to-endpoint links from a sensor to ISP is
cut between PHY and ISP, with the originally proposed device tree layout
scheme it's not a problem, if PHYs are children of the ISP.

--
Best wishes,
Vladimir

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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-06-12  1:15 ` [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices Vladimir Zapolskiy
  2025-06-12  7:43   ` Krzysztof Kozlowski
  2025-06-23  9:31   ` Neil Armstrong
@ 2025-08-07 12:37   ` Bryan O'Donoghue
  2025-08-07 14:17     ` Neil Armstrong
  2 siblings, 1 reply; 42+ messages in thread
From: Bryan O'Donoghue @ 2025-08-07 12:37 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Krzysztof Kozlowski, Rob Herring,
	Bjorn Andersson, Konrad Dybcio
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Neil Armstrong, Vinod Koul, linux-arm-msm, linux-media,
	devicetree

On 12/06/2025 02:15, Vladimir Zapolskiy wrote:
> Following the new device tree bindings for CAMSS IPs introduce csiphy2
> device tree node under SM8250 CAMSS, which allows to perform camera
> tests of the model on an RB5 board with an attached vision mezzanine.
> 
> Note that the optional 'phys' property is deliberately not added.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> For testing only, do not merge.
> 
>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index f0d18fd37aaf..401a32679580 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4613,6 +4613,10 @@ camss: camss@ac6a000 {
>   					     "cam_sf_0_mnoc",
>   					     "cam_sf_icp_mnoc";
>   
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +
>   			ports {
>   				#address-cells = <1>;
>   				#size-cells = <0>;
> @@ -4641,6 +4645,16 @@ port@5 {
>   					reg = <5>;
>   				};
>   			};
> +
> +			csiphy2: phy@ac6e000 {
> +				compatible = "qcom,csiphy";
> +				reg = <0 0x0ac6e000 0 0x1000>;
> +				clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
> +					 <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
> +				clock-names = "csiphy", "csiphy_timer";
> +				interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
> +				#phy-cells = <0>;
> +			};
>   		};
>   
I don't think we should make this change, for CAMSS in general and 
specifically for sm8250.

Instead I think we should go this way:

https://lore.kernel.org/linux-media/20250710-x1e-csi2-phy-v1-1-74acbb5b162b@linaro.org/

With separate standalone nodes, and reuse of the upstream PHY API.

I believe you have a series for the 8650, please rebase on

https://lore.kernel.org/linux-media/20250710-x1e-csi2-phy-v1-1-74acbb5b162b@linaro.org/

and

https://lore.kernel.org/linux-media/20250711-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v7-0-0bc5da82f526@linaro.org

V2 of the CSIPHY above will incorporate feedback from Neil and yourself 
on adding endpoint@ to the PHY however I think we need to have a 
conversation about standards compliance at attaching two sensors to one 
CSIPHY without VCs or TDM.

---
bod

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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-08-07 12:37   ` Bryan O'Donoghue
@ 2025-08-07 14:17     ` Neil Armstrong
  2025-08-07 15:07       ` Bryan O'Donoghue
  0 siblings, 1 reply; 42+ messages in thread
From: Neil Armstrong @ 2025-08-07 14:17 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vladimir Zapolskiy, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Vinod Koul, linux-arm-msm, linux-media, devicetree

On 07/08/2025 14:37, Bryan O'Donoghue wrote:
> On 12/06/2025 02:15, Vladimir Zapolskiy wrote:
>> Following the new device tree bindings for CAMSS IPs introduce csiphy2
>> device tree node under SM8250 CAMSS, which allows to perform camera
>> tests of the model on an RB5 board with an attached vision mezzanine.
>>
>> Note that the optional 'phys' property is deliberately not added.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>> For testing only, do not merge.
>>
>>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> index f0d18fd37aaf..401a32679580 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> @@ -4613,6 +4613,10 @@ camss: camss@ac6a000 {
>>                            "cam_sf_0_mnoc",
>>                            "cam_sf_icp_mnoc";
>> +            #address-cells = <2>;
>> +            #size-cells = <2>;
>> +            ranges;
>> +
>>               ports {
>>                   #address-cells = <1>;
>>                   #size-cells = <0>;
>> @@ -4641,6 +4645,16 @@ port@5 {
>>                       reg = <5>;
>>                   };
>>               };
>> +
>> +            csiphy2: phy@ac6e000 {
>> +                compatible = "qcom,csiphy";
>> +                reg = <0 0x0ac6e000 0 0x1000>;
>> +                clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
>> +                     <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
>> +                clock-names = "csiphy", "csiphy_timer";
>> +                interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
>> +                #phy-cells = <0>;
>> +            };
>>           };
> I don't think we should make this change, for CAMSS in general and specifically for sm8250.
> 
> Instead I think we should go this way:
> 
> https://lore.kernel.org/linux-media/20250710-x1e-csi2-phy-v1-1-74acbb5b162b@linaro.org/
> 
> With separate standalone nodes, and reuse of the upstream PHY API.
> 
> I believe you have a series for the 8650, please rebase on
> 
> https://lore.kernel.org/linux-media/20250710-x1e-csi2-phy-v1-1-74acbb5b162b@linaro.org/
> 
> and
> 
> https://lore.kernel.org/linux-media/20250711-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v7-0-0bc5da82f526@linaro.org
> 
> V2 of the CSIPHY above will incorporate feedback from Neil and yourself on adding endpoint@ to the PHY however I think we need to have a conversation about standards compliance at attaching two sensors to one CSIPHY without VCs or TDM.

The PHY is able to setup 2 lanes as clock and connect 2 sensors over the 5 lanes available, like for example:
- lane0: cam0 data0
- lane1: cam0 data1
- lane2: cam1 data0
- lane3: cam1 clk
- lane4: cam0 clk

Any lane mapping is compliant. There some Meta slides about that at:
https://www.edge-ai-vision.com/wp-content/uploads/2024/09/T2R10_Kumaran-Ayyalluseshagiri-Viswanathan_Meta_2024.pdf slide 13

Neil

> 
> ---
> bod


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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-08-07 14:17     ` Neil Armstrong
@ 2025-08-07 15:07       ` Bryan O'Donoghue
  2025-08-09  9:26         ` Dmitry Baryshkov
  0 siblings, 1 reply; 42+ messages in thread
From: Bryan O'Donoghue @ 2025-08-07 15:07 UTC (permalink / raw)
  To: Neil Armstrong, Vladimir Zapolskiy, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio
  Cc: Conor Dooley, Robert Foss, Todor Tomov, Mauro Carvalho Chehab,
	Vinod Koul, linux-arm-msm, linux-media, devicetree

On 07/08/2025 15:17, Neil Armstrong wrote:
>>
>> https://lore.kernel.org/linux-media/20250711-b4-linux-next-25-03-13- 
>> dtsi-x1e80100-camss-v7-0-0bc5da82f526@linaro.org
>>
>> V2 of the CSIPHY above will incorporate feedback from Neil and 
>> yourself on adding endpoint@ to the PHY however I think we need to 
>> have a conversation about standards compliance at attaching two 
>> sensors to one CSIPHY without VCs or TDM.
> 
> The PHY is able to setup 2 lanes as clock and connect 2 sensors over the 
> 5 lanes available, like for example:
> - lane0: cam0 data0
> - lane1: cam0 data1
> - lane2: cam1 data0
> - lane3: cam1 clk
> - lane4: cam0 clk
> 
> Any lane mapping is compliant. There some Meta slides about that at:
> https://www.edge-ai-vision.com/wp-content/uploads/2024/09/T2R10_Kumaran- 
> Ayyalluseshagiri-Viswanathan_Meta_2024.pdf slide 13

Hmm so that would require splitting the CSIPHY between two CSI decoders 
which I'm not sure would work on our hardware, perhaps yes, perhaps no, 
or routing both sensors into the one CSI decoder and then separating the 
data-streams either in the driver or in user-space.

For such an esoteric setup I think my initial suggestion would be to 
push it into user-space, even assuming you have gotten the PHY to 
co-operate with having two simultaneous clock lanes per the above link.

Looking at the PHY regs, I guess you can set the bits but obviously the 
analogue component of the PHY can only really operate from the one clock 
lane....

Interesting.

---
bod



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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-08-07 15:07       ` Bryan O'Donoghue
@ 2025-08-09  9:26         ` Dmitry Baryshkov
  2025-08-09 11:51           ` Bryan O'Donoghue
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Baryshkov @ 2025-08-09  9:26 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Neil Armstrong, Vladimir Zapolskiy, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Conor Dooley,
	Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Vinod Koul,
	linux-arm-msm, linux-media, devicetree

On Thu, Aug 07, 2025 at 04:07:24PM +0100, Bryan O'Donoghue wrote:
> On 07/08/2025 15:17, Neil Armstrong wrote:
> > > 
> > > https://lore.kernel.org/linux-media/20250711-b4-linux-next-25-03-13-
> > > dtsi-x1e80100-camss-v7-0-0bc5da82f526@linaro.org
> > > 
> > > V2 of the CSIPHY above will incorporate feedback from Neil and
> > > yourself on adding endpoint@ to the PHY however I think we need to
> > > have a conversation about standards compliance at attaching two
> > > sensors to one CSIPHY without VCs or TDM.
> > 
> > The PHY is able to setup 2 lanes as clock and connect 2 sensors over the
> > 5 lanes available, like for example:
> > - lane0: cam0 data0
> > - lane1: cam0 data1
> > - lane2: cam1 data0
> > - lane3: cam1 clk
> > - lane4: cam0 clk
> > 
> > Any lane mapping is compliant. There some Meta slides about that at:
> > https://www.edge-ai-vision.com/wp-content/uploads/2024/09/T2R10_Kumaran-
> > Ayyalluseshagiri-Viswanathan_Meta_2024.pdf slide 13
> 
> Hmm so that would require splitting the CSIPHY between two CSI decoders
> which I'm not sure would work on our hardware, perhaps yes, perhaps no, or
> routing both sensors into the one CSI decoder and then separating the
> data-streams either in the driver or in user-space.

The RB5 board provides exactly this setup on the CSI0. It can be
switched between 4 lanes going to CSI0A and 2 (data) lanes going to
the CSI0A and 1 (data) lane going to the CSI0B connector.

> For such an esoteric setup I think my initial suggestion would be to push it
> into user-space, even assuming you have gotten the PHY to co-operate with
> having two simultaneous clock lanes per the above link.
> 
> Looking at the PHY regs, I guess you can set the bits but obviously the
> analogue component of the PHY can only really operate from the one clock
> lane....
> 
> Interesting.
> 
> ---
> bod
> 
> 

-- 
With best wishes
Dmitry

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

* Re: [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices
  2025-08-09  9:26         ` Dmitry Baryshkov
@ 2025-08-09 11:51           ` Bryan O'Donoghue
  0 siblings, 0 replies; 42+ messages in thread
From: Bryan O'Donoghue @ 2025-08-09 11:51 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Neil Armstrong, Vladimir Zapolskiy, Krzysztof Kozlowski,
	Rob Herring, Bjorn Andersson, Konrad Dybcio, Conor Dooley,
	Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Vinod Koul,
	linux-arm-msm, linux-media, devicetree

On 09/08/2025 10:26, Dmitry Baryshkov wrote:
>> Hmm so that would require splitting the CSIPHY between two CSI decoders
>> which I'm not sure would work on our hardware, perhaps yes, perhaps no, or
>> routing both sensors into the one CSI decoder and then separating the
>> data-streams either in the driver or in user-space.
> The RB5 board provides exactly this setup on the CSI0. It can be
> switched between 4 lanes going to CSI0A and 2 (data) lanes going to
> the CSI0A and 1 (data) lane going to the CSI0B connector.

Yes I see that.

I see the CSID can select which lanes it maps. So you could have one 
CSID map lanes 1-2 and another CSID map lane 4.

Still I don't see how you could have two sensors at different data-rates 
in this setup since what type of frame would the PHY present on the 
internal bus..

It doesn't really matter if its a standards compliant setup if the 
hardware supports it either.

Still though there's no reason to have a custom PHY driver continue to 
live in CAMSS.

---
bod

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

* Re: [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-06-12  1:15 ` [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
  2025-06-12  7:25   ` Krzysztof Kozlowski
  2025-06-12  7:38   ` Krzysztof Kozlowski
@ 2025-08-11 10:53   ` Dmitry Baryshkov
  2 siblings, 0 replies; 42+ messages in thread
From: Dmitry Baryshkov @ 2025-08-11 10:53 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Konrad Dybcio,
	Bryan O'Donoghue, Conor Dooley, Robert Foss, Todor Tomov,
	Mauro Carvalho Chehab, Neil Armstrong, Vinod Koul, linux-arm-msm,
	linux-media, devicetree

On Thu, Jun 12, 2025 at 04:15:29AM +0300, Vladimir Zapolskiy wrote:
> Add dt-binding schema for Qualcomm CAMSS CSIPHY IP, which provides
> MIPI C-PHY/D-PHY interfaces on Qualcomm SoCs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> RFC verion of the change:
> * https://lore.kernel.org/all/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/
> 
> Changes from RFC to v1:
> * moved from phy/qcom,csiphy.yaml to media/qcom,csiphy.yaml,
> * added 'clock-names' property,
> * removed SM8250 CSIPHY specifics, a generic binding is good enough for now,
> * the above implies a removal of SM8250 specific supplies.
> 
>  .../bindings/media/qcom,csiphy.yaml           | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/qcom,csiphy.yaml
> 
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    description: CAMSS CSIPHY input port
> +
> +    patternProperties:
> +      "^endpoint@[0-1]$":
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +          bus-type:
> +            enum:
> +              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> +              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> +        required:
> +          - data-lanes

 - clock-lanes

> +

-- 
With best wishes
Dmitry

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

end of thread, other threads:[~2025-08-11 10:53 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-12  1:15 [PATCH 00/10] media: qcom: camss: add support for csiphy devices Vladimir Zapolskiy
2025-06-12  1:15 ` [PATCH 01/10] media: qcom: camss: remove never used camss_vfe_get()/camss_vfe_put() Vladimir Zapolskiy
2025-06-12  7:31   ` Bryan O'Donoghue
2025-06-12  1:15 ` [PATCH 02/10] media: qcom: camss: remove subdev resource argument from msm_csiphy_subdev_init() Vladimir Zapolskiy
2025-06-12  7:37   ` Bryan O'Donoghue
2025-06-12  1:15 ` [PATCH 03/10] media: qcom: camss: csiphy: simplify arguments of lanes_enable and lanes_disable Vladimir Zapolskiy
2025-06-12  8:02   ` Bryan O'Donoghue
2025-06-12  1:15 ` [PATCH 04/10] media: qcom: camss: populate CAMSS children subdevices of CSIPHY IPs Vladimir Zapolskiy
2025-06-12  1:15 ` [PATCH 05/10] media: qcom: camss: unwrap platform driver registration Vladimir Zapolskiy
2025-06-12  1:15 ` [PATCH 06/10] media: qcom: camss: export camss_parse_endpoint_node() to csiphy Vladimir Zapolskiy
2025-06-12  1:15 ` [PATCH 07/10] media: qcom: camss: csiphy: probe any present children CSIPHY subdevices Vladimir Zapolskiy
2025-06-12  1:15 ` [PATCH 08/10] dt-bindings: media: qcom: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
2025-06-12  7:25   ` Krzysztof Kozlowski
2025-06-12  7:38   ` Krzysztof Kozlowski
2025-06-12  7:39     ` Krzysztof Kozlowski
2025-06-12  7:57       ` Vladimir Zapolskiy
2025-06-12 11:02         ` Krzysztof Kozlowski
2025-06-12 11:27           ` Vladimir Zapolskiy
2025-06-12 11:36             ` Krzysztof Kozlowski
2025-06-12 16:17         ` Konrad Dybcio
2025-06-12 16:44           ` Vladimir Zapolskiy
2025-06-14 19:29             ` Konrad Dybcio
2025-06-12 17:13     ` Vladimir Zapolskiy
2025-06-13  6:28       ` Krzysztof Kozlowski
2025-06-14 19:31         ` Konrad Dybcio
2025-06-17  6:32           ` Krzysztof Kozlowski
2025-06-17  9:51             ` Vladimir Zapolskiy
2025-06-17 19:20               ` Konrad Dybcio
2025-06-17 21:30                 ` Vladimir Zapolskiy
2025-08-11 10:53   ` Dmitry Baryshkov
2025-06-12  1:15 ` [PATCH 09/10] [RFT] arm64: dts: qcom: sm8250: extend CAMSS with new CSIPHY subdevices Vladimir Zapolskiy
2025-06-12  7:43   ` Krzysztof Kozlowski
2025-06-12 16:25     ` Konrad Dybcio
2025-06-12 17:03     ` Vladimir Zapolskiy
2025-06-23  9:31   ` Neil Armstrong
2025-06-23 13:06     ` Vladimir Zapolskiy
2025-08-07 12:37   ` Bryan O'Donoghue
2025-08-07 14:17     ` Neil Armstrong
2025-08-07 15:07       ` Bryan O'Donoghue
2025-08-09  9:26         ` Dmitry Baryshkov
2025-08-09 11:51           ` Bryan O'Donoghue
2025-06-12  1:15 ` [PATCH 10/10] [RFT] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: switch to new CSIPHY scheme Vladimir Zapolskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).