mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/8] media: stm32: csi / dcmipp corrections
@ 2025-01-08 13:16 Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 1/8] dt-bindings: media: clarify stm32 csi & simplify example Alain Volmat
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Various fixes within the stm32 csi bindings/drivers and
stm32 dcmipp driver.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
Changes in v2:
- Correct commit message within the dt-binding commit, related to
  removal of bus-type property from the example.
- Link to v1: https://lore.kernel.org/r/20241217-csi_dcmipp_mp25_enhancements-v1-0-2b432805d17d@foss.st.com

---
Alain Volmat (8):
      dt-bindings: media: clarify stm32 csi & simplify example
      media: stm32: csi: add missing pm_runtime_put on error
      media: stm32: csi: register subdev only at end of probe
      media: stm32: csi: use ARRAY_SIZE to search D-PHY table
      media: stm32: csi: simplify enable_streams error handling
      media: stm32: csi: remove useless fwnode_graph_get_endpoint call
      media: stm32: csi: correct unsigned or useless variable settings
      media: stm32: dcmipp: add has_csi2 & needs_mclk in match data

 .../bindings/media/st,stm32mp25-csi.yaml           |   5 +-
 drivers/media/platform/st/stm32/stm32-csi.c        | 102 +++++++++++----------
 .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   |  23 +++--
 3 files changed, 67 insertions(+), 63 deletions(-)
---
base-commit: 0e420e3a0a9a0d8d0b9d3d262baa240ef2369c88
change-id: 20241217-csi_dcmipp_mp25_enhancements-89e0eef194bf

Best regards,
-- 
Alain Volmat <alain.volmat@foss.st.com>


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

* [PATCH v2 1/8] dt-bindings: media: clarify stm32 csi & simplify example
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-10 16:02   ` Rob Herring (Arm)
  2025-01-08 13:16 ` [PATCH v2 2/8] media: stm32: csi: add missing pm_runtime_put on error Alain Volmat
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Clarify the description of the stm32 CSI by mentioning CSI-2 and D-PHY.
Remove the bus-type property from the example since this CSI has a D-PHY,
which is the only bus-type option, making this property redundant.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

---
v2: clarify commit message related to bus-type property removal within
    the example
---
 Documentation/devicetree/bindings/media/st,stm32mp25-csi.yaml | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/st,stm32mp25-csi.yaml b/Documentation/devicetree/bindings/media/st,stm32mp25-csi.yaml
index 33bedfe41924..e9fa3cfea5d2 100644
--- a/Documentation/devicetree/bindings/media/st,stm32mp25-csi.yaml
+++ b/Documentation/devicetree/bindings/media/st,stm32mp25-csi.yaml
@@ -7,8 +7,8 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: STMicroelectronics STM32 CSI controller
 
 description:
-  The STM32 CSI controller allows connecting a CSI based
-  camera to the DCMIPP camera pipeline.
+  The STM32 CSI controller, coupled with a D-PHY allows connecting a CSI-2
+  based camera to the DCMIPP camera pipeline.
 
 maintainers:
   - Alain Volmat <alain.volmat@foss.st.com>
@@ -109,7 +109,6 @@ examples:
                 endpoint {
                     remote-endpoint = <&imx335_ep>;
                     data-lanes = <1 2>;
-                    bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
                 };
             };
 

-- 
2.34.1


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

* [PATCH v2 2/8] media: stm32: csi: add missing pm_runtime_put on error
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 1/8] dt-bindings: media: clarify stm32 csi & simplify example Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 3/8] media: stm32: csi: register subdev only at end of probe Alain Volmat
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Within the stm32_csi_start function, pm_runtime_put should
be called upon error following pm_runtime_get_sync.
Rework the function error handling by putting a label in
order to have common error handling for all calls requiring
pm_runtime_put.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/media/platform/st/stm32/stm32-csi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-csi.c b/drivers/media/platform/st/stm32/stm32-csi.c
index 48941aae8c9b..e2f63bb47d33 100644
--- a/drivers/media/platform/st/stm32/stm32-csi.c
+++ b/drivers/media/platform/st/stm32/stm32-csi.c
@@ -497,21 +497,19 @@ static int stm32_csi_start(struct stm32_csi_dev *csidev,
 
 	ret = pm_runtime_get_sync(csidev->dev);
 	if (ret < 0)
-		return ret;
+		goto error_put;
 
 	/* Retrieve CSI2PHY clock rate to compute CCFR value */
 	phy_clk_frate = clk_get_rate(csidev->clks[STM32_CSI_CLK_CSI2PHY].clk);
 	if (!phy_clk_frate) {
-		pm_runtime_put(csidev->dev);
 		dev_err(csidev->dev, "CSI2PHY clock rate invalid (0)\n");
-		return ret;
+		ret = -EINVAL;
+		goto error_put;
 	}
 
 	ret = stm32_csi_setup_lane_merger(csidev);
-	if (ret) {
-		pm_runtime_put(csidev->dev);
-		return ret;
-	}
+	if (ret)
+		goto error_put;
 
 	/* Enable the CSI */
 	writel_relaxed(STM32_CSI_CR_CSIEN, csidev->base + STM32_CSI_CR);
@@ -567,6 +565,10 @@ static int stm32_csi_start(struct stm32_csi_dev *csidev,
 	writel_relaxed(0, csidev->base + STM32_CSI_PMCR);
 
 	return ret;
+
+error_put:
+	pm_runtime_put(csidev->dev);
+	return ret;
 }
 
 static void stm32_csi_stop(struct stm32_csi_dev *csidev)

-- 
2.34.1


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

* [PATCH v2 3/8] media: stm32: csi: register subdev only at end of probe
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 1/8] dt-bindings: media: clarify stm32 csi & simplify example Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 2/8] media: stm32: csi: add missing pm_runtime_put on error Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 4/8] media: stm32: csi: use ARRAY_SIZE to search D-PHY table Alain Volmat
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Call v4l2_async_register_subdev only whenever all initialization
are completed at the end of the probe function.
Remove as well useless err_free_priv label by returning directly
upon error.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/media/platform/st/stm32/stm32-csi.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-csi.c b/drivers/media/platform/st/stm32/stm32-csi.c
index e2f63bb47d33..89fcd7d07904 100644
--- a/drivers/media/platform/st/stm32/stm32-csi.c
+++ b/drivers/media/platform/st/stm32/stm32-csi.c
@@ -991,11 +991,11 @@ static int stm32_csi_probe(struct platform_device *pdev)
 
 	ret = stm32_csi_get_resources(csidev, pdev);
 	if (ret)
-		goto err_free_priv;
+		return ret;
 
 	ret = stm32_csi_parse_dt(csidev);
 	if (ret)
-		goto err_free_priv;
+		return ret;
 
 	csidev->sd.owner = THIS_MODULE;
 	csidev->sd.dev = &pdev->dev;
@@ -1020,10 +1020,6 @@ static int stm32_csi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_cleanup;
 
-	ret = v4l2_async_register_subdev(&csidev->sd);
-	if (ret < 0)
-		goto err_cleanup;
-
 	/* Reset device */
 	rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
 	if (IS_ERR(rstc)) {
@@ -1050,6 +1046,10 @@ static int stm32_csi_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 
+	ret = v4l2_async_register_subdev(&csidev->sd);
+	if (ret < 0)
+		goto err_cleanup;
+
 	dev_info(&pdev->dev,
 		 "Probed CSI with %u lanes\n", csidev->num_lanes);
 
@@ -1057,7 +1057,6 @@ static int stm32_csi_probe(struct platform_device *pdev)
 
 err_cleanup:
 	v4l2_async_nf_cleanup(&csidev->notifier);
-err_free_priv:
 	return ret;
 }
 

-- 
2.34.1


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

* [PATCH v2 4/8] media: stm32: csi: use ARRAY_SIZE to search D-PHY table
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
                   ` (2 preceding siblings ...)
  2025-01-08 13:16 ` [PATCH v2 3/8] media: stm32: csi: register subdev only at end of probe Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 5/8] media: stm32: csi: simplify enable_streams error handling Alain Volmat
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Within stm32_csi_start, use ARRAY_SIZE loop in order to search
for the right setting.
Avoid useless init of lanes_ie / lanes_en.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/media/platform/st/stm32/stm32-csi.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-csi.c b/drivers/media/platform/st/stm32/stm32-csi.c
index 89fcd7d07904..3654f9895dbc 100644
--- a/drivers/media/platform/st/stm32/stm32-csi.c
+++ b/drivers/media/platform/st/stm32/stm32-csi.c
@@ -325,7 +325,6 @@ static const struct stm32_csi_mbps_phy_reg snps_stm32mp25[] = {
 	{ .mbps = 2400, .hsfreqrange = 0x47,	.osc_freq_target = 442 },
 	{ .mbps = 2450, .hsfreqrange = 0x48,	.osc_freq_target = 451 },
 	{ .mbps = 2500, .hsfreqrange = 0x49,	.osc_freq_target = 460 },
-	{ /* sentinel */ }
 };
 
 static const struct v4l2_mbus_framefmt fmt_default = {
@@ -444,13 +443,13 @@ static void stm32_csi_phy_reg_write(struct stm32_csi_dev *csidev,
 static int stm32_csi_start(struct stm32_csi_dev *csidev,
 			   struct v4l2_subdev_state *state)
 {
-	const struct stm32_csi_mbps_phy_reg *phy_regs;
+	const struct stm32_csi_mbps_phy_reg *phy_regs = NULL;
 	struct v4l2_mbus_framefmt *sink_fmt;
 	const struct stm32_csi_fmts *fmt;
 	unsigned long phy_clk_frate;
+	u32 lanes_ie, lanes_en;
 	unsigned int mbps;
-	u32 lanes_ie = 0;
-	u32 lanes_en = 0;
+	unsigned int i;
 	s64 link_freq;
 	int ret;
 	u32 ccfr;
@@ -474,11 +473,14 @@ static int stm32_csi_start(struct stm32_csi_dev *csidev,
 	mbps = div_s64(link_freq, 500000);
 	dev_dbg(csidev->dev, "Computed Mbps: %u\n", mbps);
 
-	for (phy_regs = snps_stm32mp25; phy_regs->mbps != 0; phy_regs++)
-		if (phy_regs->mbps >= mbps)
+	for (i = 0; i < ARRAY_SIZE(snps_stm32mp25); i++) {
+		if (snps_stm32mp25[i].mbps >= mbps) {
+			phy_regs = &snps_stm32mp25[i];
 			break;
+		}
+	}
 
-	if (!phy_regs->mbps) {
+	if (!phy_regs) {
 		dev_err(csidev->dev, "Unsupported PHY speed (%u Mbps)", mbps);
 		return -ERANGE;
 	}
@@ -488,8 +490,8 @@ static int stm32_csi_start(struct stm32_csi_dev *csidev,
 		phy_regs->osc_freq_target);
 
 	/* Prepare lanes related configuration bits */
-	lanes_ie |= STM32_CSI_SR1_DL0_ERRORS;
-	lanes_en |= STM32_CSI_PCR_DL0EN;
+	lanes_ie = STM32_CSI_SR1_DL0_ERRORS;
+	lanes_en = STM32_CSI_PCR_DL0EN;
 	if (csidev->num_lanes == 2) {
 		lanes_ie |= STM32_CSI_SR1_DL1_ERRORS;
 		lanes_en |= STM32_CSI_PCR_DL1EN;

-- 
2.34.1


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

* [PATCH v2 5/8] media: stm32: csi: simplify enable_streams error handling
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
                   ` (3 preceding siblings ...)
  2025-01-08 13:16 ` [PATCH v2 4/8] media: stm32: csi: use ARRAY_SIZE to search D-PHY table Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 6/8] media: stm32: csi: remove useless fwnode_graph_get_endpoint call Alain Volmat
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Put all error handling for VC stop and CSI stop together
to avoid duplication of code.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/media/platform/st/stm32/stm32-csi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-csi.c b/drivers/media/platform/st/stm32/stm32-csi.c
index 3654f9895dbc..4affbc00b042 100644
--- a/drivers/media/platform/st/stm32/stm32-csi.c
+++ b/drivers/media/platform/st/stm32/stm32-csi.c
@@ -694,19 +694,21 @@ static int stm32_csi_enable_streams(struct v4l2_subdev *sd,
 	ret = stm32_csi_start_vc(csidev, state, 0);
 	if (ret) {
 		dev_err(csidev->dev, "Failed to start VC0\n");
-		stm32_csi_stop(csidev);
-		return ret;
+		goto failed_start_vc;
 	}
 
 	ret = v4l2_subdev_enable_streams(csidev->s_subdev,
 					 csidev->s_subdev_pad_nb, BIT_ULL(0));
-	if (ret) {
-		stm32_csi_stop_vc(csidev, 0);
-		stm32_csi_stop(csidev);
-		return ret;
-	}
+	if (ret)
+		goto failed_enable_streams;
 
 	return 0;
+
+failed_enable_streams:
+	stm32_csi_stop_vc(csidev, 0);
+failed_start_vc:
+	stm32_csi_stop(csidev);
+	return ret;
 }
 
 static int stm32_csi_init_state(struct v4l2_subdev *sd,

-- 
2.34.1


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

* [PATCH v2 6/8] media: stm32: csi: remove useless fwnode_graph_get_endpoint call
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
                   ` (4 preceding siblings ...)
  2025-01-08 13:16 ` [PATCH v2 5/8] media: stm32: csi: simplify enable_streams error handling Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 7/8] media: stm32: csi: correct unsigned or useless variable settings Alain Volmat
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

The endpoint is already retrieved at the beginning of the function
stm32_csi_parse_dt hence keep the endpoint pointer until the end
instead of getting a new one.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/media/platform/st/stm32/stm32-csi.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-csi.c b/drivers/media/platform/st/stm32/stm32-csi.c
index 4affbc00b042..1be12c9dcf55 100644
--- a/drivers/media/platform/st/stm32/stm32-csi.c
+++ b/drivers/media/platform/st/stm32/stm32-csi.c
@@ -932,38 +932,32 @@ static int stm32_csi_parse_dt(struct stm32_csi_dev *csidev)
 	}
 
 	ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
-	fwnode_handle_put(ep);
 	if (ret) {
 		dev_err(csidev->dev, "Could not parse v4l2 endpoint\n");
-		return ret;
+		goto out;
 	}
 
 	csidev->num_lanes = v4l2_ep.bus.mipi_csi2.num_data_lanes;
 	if (csidev->num_lanes > STM32_CSI_LANES_MAX) {
 		dev_err(csidev->dev, "Unsupported number of data-lanes: %d\n",
 			csidev->num_lanes);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	memcpy(csidev->lanes, v4l2_ep.bus.mipi_csi2.data_lanes,
 	       sizeof(csidev->lanes));
 
-	ep = fwnode_graph_get_next_endpoint(dev_fwnode(csidev->dev), NULL);
-	if (!ep) {
-		dev_err(csidev->dev, "Failed to get next endpoint\n");
-		return -EINVAL;
-	}
-
 	v4l2_async_subdev_nf_init(&csidev->notifier, &csidev->sd);
 
 	asd = v4l2_async_nf_add_fwnode_remote(&csidev->notifier, ep,
 					      struct v4l2_async_connection);
 
-	fwnode_handle_put(ep);
 
 	if (IS_ERR(asd)) {
 		dev_err(csidev->dev, "Failed to add fwnode remote subdev\n");
-		return PTR_ERR(asd);
+		ret = PTR_ERR(asd);
+		goto out;
 	}
 
 	csidev->notifier.ops = &stm32_csi_notifier_ops;
@@ -972,9 +966,11 @@ static int stm32_csi_parse_dt(struct stm32_csi_dev *csidev)
 	if (ret) {
 		dev_err(csidev->dev, "Failed to register notifier\n");
 		v4l2_async_nf_cleanup(&csidev->notifier);
-		return ret;
+		goto out;
 	}
 
+out:
+	fwnode_handle_put(ep);
 	return ret;
 }
 

-- 
2.34.1


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

* [PATCH v2 7/8] media: stm32: csi: correct unsigned or useless variable settings
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
                   ` (5 preceding siblings ...)
  2025-01-08 13:16 ` [PATCH v2 6/8] media: stm32: csi: remove useless fwnode_graph_get_endpoint call Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-08 13:16 ` [PATCH v2 8/8] media: stm32: dcmipp: add has_csi2 & needs_mclk in match data Alain Volmat
  2025-01-10  8:08 ` [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Sakari Ailus
  8 siblings, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Correct several missing unsigned type missing for loop variables
and also remove useless initialization of variables.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/media/platform/st/stm32/stm32-csi.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-csi.c b/drivers/media/platform/st/stm32/stm32-csi.c
index 1be12c9dcf55..602b0879f21e 100644
--- a/drivers/media/platform/st/stm32/stm32-csi.c
+++ b/drivers/media/platform/st/stm32/stm32-csi.c
@@ -357,7 +357,7 @@ static inline struct stm32_csi_dev *to_csidev(struct v4l2_subdev *sd)
 static int stm32_csi_setup_lane_merger(struct stm32_csi_dev *csidev)
 {
 	u32 lmcfgr = 0;
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < csidev->num_lanes; i++) {
 		if (!csidev->lanes[i] || csidev->lanes[i] > STM32_CSI_LANES_MAX) {
@@ -595,20 +595,20 @@ static int stm32_csi_start_vc(struct stm32_csi_dev *csidev,
 {
 	struct v4l2_mbus_framefmt *mbus_fmt;
 	const struct stm32_csi_fmts *fmt;
-	u32 cfgr1 = 0;
-	int ret = 0;
 	u32 status;
+	u32 cfgr1;
+	int ret;
 
 	mbus_fmt = v4l2_subdev_state_get_format(state, STM32_CSI_PAD_SOURCE);
 	fmt = stm32_csi_code_to_fmt(mbus_fmt->code);
 
 	/* If the mbus code is JPEG, don't enable filtering */
 	if (mbus_fmt->code == MEDIA_BUS_FMT_JPEG_1X8) {
-		cfgr1 |= STM32_CSI_VCXCFGR1_ALLDT;
+		cfgr1 = STM32_CSI_VCXCFGR1_ALLDT;
 		cfgr1 |= fmt->input_fmt << STM32_CSI_VCXCFGR1_CDTFT_SHIFT;
 		dev_dbg(csidev->dev, "VC%d: enable AllDT mode\n", vc);
 	} else {
-		cfgr1 |= fmt->datatype << STM32_CSI_VCXCFGR1_DT0_SHIFT;
+		cfgr1 = fmt->datatype << STM32_CSI_VCXCFGR1_DT0_SHIFT;
 		cfgr1 |= fmt->input_fmt << STM32_CSI_VCXCFGR1_DT0FT_SHIFT;
 		cfgr1 |= STM32_CSI_VCXCFGR1_DT0EN;
 		dev_dbg(csidev->dev, "VC%d: enable DT0(0x%x)/DT0FT(0x%x)\n",
@@ -634,8 +634,8 @@ static int stm32_csi_start_vc(struct stm32_csi_dev *csidev,
 
 static int stm32_csi_stop_vc(struct stm32_csi_dev *csidev, u32 vc)
 {
-	int ret = 0;
 	u32 status;
+	int ret;
 
 	/* Stop the Virtual Channel */
 	writel_relaxed(STM32_CSI_CR_VCXSTOP(vc) | STM32_CSI_CR_CSIEN,
@@ -714,7 +714,7 @@ static int stm32_csi_enable_streams(struct v4l2_subdev *sd,
 static int stm32_csi_init_state(struct v4l2_subdev *sd,
 				struct v4l2_subdev_state *state)
 {
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < sd->entity.num_pads; i++)
 		*v4l2_subdev_state_get_format(state, i) = fmt_default;
@@ -879,7 +879,8 @@ static irqreturn_t stm32_csi_irq_thread(int irq, void *arg)
 static int stm32_csi_get_resources(struct stm32_csi_dev *csidev,
 				   struct platform_device *pdev)
 {
-	int irq, ret, i;
+	unsigned int i;
+	int irq, ret;
 
 	csidev->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
 	if (IS_ERR(csidev->base))

-- 
2.34.1


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

* [PATCH v2 8/8] media: stm32: dcmipp: add has_csi2 & needs_mclk in match data
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
                   ` (6 preceding siblings ...)
  2025-01-08 13:16 ` [PATCH v2 7/8] media: stm32: csi: correct unsigned or useless variable settings Alain Volmat
@ 2025-01-08 13:16 ` Alain Volmat
  2025-01-10  8:08 ` [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Sakari Ailus
  8 siblings, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-08 13:16 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Sakari Ailus, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-media, linux-stm32, linux-arm-kernel, linux-kernel,
	devicetree, Alain Volmat

Introduce two variable has_csi and has_mclk within the
match data of the driver in order to know, depending on
the compatible if CSI-2 interface is available and if
the mclk clk should be retrieved.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 .../platform/st/stm32/stm32-dcmipp/dcmipp-core.c   | 23 ++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
index 5a04018a6a9d..1b7bae3266c8 100644
--- a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
+++ b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
@@ -89,6 +89,8 @@ struct dcmipp_pipeline_config {
 	const struct dcmipp_ent_link *links;
 	size_t num_links;
 	u32 hw_revision;
+	bool has_csi2;
+	bool needs_mclk;
 };
 
 /* --------------------------------------------------------------------------
@@ -164,7 +166,9 @@ static const struct dcmipp_pipeline_config stm32mp25_pipe_cfg = {
 	.num_ents	= ARRAY_SIZE(stm32mp25_ent_config),
 	.links		= stm32mp25_ent_links,
 	.num_links	= ARRAY_SIZE(stm32mp25_ent_links),
-	.hw_revision    = DCMIPP_STM32MP25_VERR
+	.hw_revision    = DCMIPP_STM32MP25_VERR,
+	.has_csi2	= true,
+	.needs_mclk	= true
 };
 
 #define LINK_FLAG_TO_STR(f) ((f) == 0 ? "" :\
@@ -296,7 +300,7 @@ static int dcmipp_graph_notify_bound(struct v4l2_async_notifier *notifier,
 				     struct v4l2_async_connection *asd)
 {
 	struct dcmipp_device *dcmipp = notifier_to_dcmipp(notifier);
-	int ret;
+	int ret = -EINVAL;
 	int src_pad, i;
 	struct dcmipp_ent_device *sink;
 	struct v4l2_fwnode_endpoint vep = { 0 };
@@ -304,15 +308,9 @@ static int dcmipp_graph_notify_bound(struct v4l2_async_notifier *notifier,
 	enum v4l2_mbus_type supported_types[] = {
 		V4L2_MBUS_PARALLEL, V4L2_MBUS_BT656, V4L2_MBUS_CSI2_DPHY
 	};
-	int supported_types_nb = ARRAY_SIZE(supported_types);
 
 	dev_dbg(dcmipp->dev, "Subdev \"%s\" bound\n", subdev->name);
 
-	/* Only MP25 supports CSI input */
-	if (!of_device_is_compatible(dcmipp->dev->of_node,
-				     "st,stm32mp25-dcmipp"))
-		supported_types_nb--;
-
 	/*
 	 * Link this sub-device to DCMIPP, it could be
 	 * a parallel camera sensor or a CSI-2 to parallel bridge
@@ -330,7 +328,12 @@ static int dcmipp_graph_notify_bound(struct v4l2_async_notifier *notifier,
 	}
 
 	/* Check for supported MBUS type */
-	for (i = 0; i < supported_types_nb; i++) {
+	for (i = 0; i < ARRAY_SIZE(supported_types); i++) {
+		/* Only MP25 supports CSI input */
+		if (supported_types[i] == V4L2_MBUS_CSI2_DPHY &&
+		    !dcmipp->pipe_cfg->has_csi2)
+			continue;
+
 		vep.bus_type = supported_types[i];
 		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
 		if (!ret)
@@ -529,7 +532,7 @@ static int dcmipp_probe(struct platform_device *pdev)
 				     "Unable to get kclk\n");
 	dcmipp->kclk = kclk;
 
-	if (!of_device_is_compatible(pdev->dev.of_node, "st,stm32mp13-dcmipp")) {
+	if (dcmipp->pipe_cfg->needs_mclk) {
 		mclk = devm_clk_get(&pdev->dev, "mclk");
 		if (IS_ERR(mclk))
 			return dev_err_probe(&pdev->dev, PTR_ERR(mclk),

-- 
2.34.1


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

* Re: [PATCH v2 0/8] media: stm32: csi / dcmipp corrections
  2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
                   ` (7 preceding siblings ...)
  2025-01-08 13:16 ` [PATCH v2 8/8] media: stm32: dcmipp: add has_csi2 & needs_mclk in match data Alain Volmat
@ 2025-01-10  8:08 ` Sakari Ailus
  2025-01-10  8:10   ` Sakari Ailus
  2025-01-13  8:55   ` Alain Volmat
  8 siblings, 2 replies; 13+ messages in thread
From: Sakari Ailus @ 2025-01-10  8:08 UTC (permalink / raw)
  To: Alain Volmat
  Cc: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-media, linux-stm32, linux-arm-kernel,
	linux-kernel, devicetree

Hi Alain,

On Wed, Jan 08, 2025 at 02:16:03PM +0100, Alain Volmat wrote:
> Various fixes within the stm32 csi bindings/drivers and
> stm32 dcmipp driver.
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Thanks for the update. I tried to apply this but it doesn't, and the
culprit appears to be this line in dcmipp_graph_notify_bound():

	unsigned int ret;

That appears to be a bug as such. The interesting thing is that the patch
changing things around it presumes

	int ret;

so it won't apply. Do you already have a patch that fixes it? It should be
cc'd to stable, too, with a Fixes: tag.

-- 
Regards,

Sakari Ailus

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

* Re: [PATCH v2 0/8] media: stm32: csi / dcmipp corrections
  2025-01-10  8:08 ` [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Sakari Ailus
@ 2025-01-10  8:10   ` Sakari Ailus
  2025-01-13  8:55   ` Alain Volmat
  1 sibling, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2025-01-10  8:10 UTC (permalink / raw)
  To: Alain Volmat
  Cc: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-media, linux-stm32, linux-arm-kernel,
	linux-kernel, devicetree

On Fri, Jan 10, 2025 at 08:08:47AM +0000, Sakari Ailus wrote:
> Hi Alain,
> 
> On Wed, Jan 08, 2025 at 02:16:03PM +0100, Alain Volmat wrote:
> > Various fixes within the stm32 csi bindings/drivers and
> > stm32 dcmipp driver.
> > 
> > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> 
> Thanks for the update. I tried to apply this but it doesn't, and the
> culprit appears to be this line in dcmipp_graph_notify_bound():
> 
> 	unsigned int ret;
> 
> That appears to be a bug as such. The interesting thing is that the patch
> changing things around it presumes
> 
> 	int ret;
> 
> so it won't apply. Do you already have a patch that fixes it? It should be
> cc'd to stable, too, with a Fixes: tag.

Ok, the tag isn't necessary as there lack of the sign doesn't seem to
matter in this case.

-- 
Sakari Ailus

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

* Re: [PATCH v2 1/8] dt-bindings: media: clarify stm32 csi & simplify example
  2025-01-08 13:16 ` [PATCH v2 1/8] dt-bindings: media: clarify stm32 csi & simplify example Alain Volmat
@ 2025-01-10 16:02   ` Rob Herring (Arm)
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring (Arm) @ 2025-01-10 16:02 UTC (permalink / raw)
  To: Alain Volmat
  Cc: linux-media, Hans Verkuil, linux-kernel, linux-arm-kernel,
	Maxime Coquelin, Alexandre Torgue, devicetree, Hugues Fruchet,
	Krzysztof Kozlowski, Mauro Carvalho Chehab, Conor Dooley,
	Sakari Ailus, linux-stm32


On Wed, 08 Jan 2025 14:16:04 +0100, Alain Volmat wrote:
> Clarify the description of the stm32 CSI by mentioning CSI-2 and D-PHY.
> Remove the bus-type property from the example since this CSI has a D-PHY,
> which is the only bus-type option, making this property redundant.
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> 
> ---
> v2: clarify commit message related to bus-type property removal within
>     the example
> ---
>  Documentation/devicetree/bindings/media/st,stm32mp25-csi.yaml | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


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

* Re: [PATCH v2 0/8] media: stm32: csi / dcmipp corrections
  2025-01-10  8:08 ` [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Sakari Ailus
  2025-01-10  8:10   ` Sakari Ailus
@ 2025-01-13  8:55   ` Alain Volmat
  1 sibling, 0 replies; 13+ messages in thread
From: Alain Volmat @ 2025-01-13  8:55 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Hugues Fruchet, Mauro Carvalho Chehab, Maxime Coquelin,
	Alexandre Torgue, Hans Verkuil, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-media, linux-stm32, linux-arm-kernel,
	linux-kernel, devicetree

Hi Sakari,

On Fri, Jan 10, 2025 at 08:08:47AM +0000, Sakari Ailus wrote:
> Hi Alain,
> 
> On Wed, Jan 08, 2025 at 02:16:03PM +0100, Alain Volmat wrote:
> > Various fixes within the stm32 csi bindings/drivers and
> > stm32 dcmipp driver.
> > 
> > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> 
> Thanks for the update. I tried to apply this but it doesn't, and the
> culprit appears to be this line in dcmipp_graph_notify_bound():
> 
> 	unsigned int ret;
> 
> That appears to be a bug as such. The interesting thing is that the patch
> changing things around it presumes
> 
> 	int ret;
> 
> so it won't apply. Do you already have a patch that fixes it? It should be
> cc'd to stable, too, with a Fixes: tag.

Apologies for that, I might have made a mistake while rebase and somehow
a patch went before the b4 header commit. Pushing again a v3 with the
missing commit added.

Regards,
Alain

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

end of thread, other threads:[~2025-01-13  8:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-08 13:16 [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Alain Volmat
2025-01-08 13:16 ` [PATCH v2 1/8] dt-bindings: media: clarify stm32 csi & simplify example Alain Volmat
2025-01-10 16:02   ` Rob Herring (Arm)
2025-01-08 13:16 ` [PATCH v2 2/8] media: stm32: csi: add missing pm_runtime_put on error Alain Volmat
2025-01-08 13:16 ` [PATCH v2 3/8] media: stm32: csi: register subdev only at end of probe Alain Volmat
2025-01-08 13:16 ` [PATCH v2 4/8] media: stm32: csi: use ARRAY_SIZE to search D-PHY table Alain Volmat
2025-01-08 13:16 ` [PATCH v2 5/8] media: stm32: csi: simplify enable_streams error handling Alain Volmat
2025-01-08 13:16 ` [PATCH v2 6/8] media: stm32: csi: remove useless fwnode_graph_get_endpoint call Alain Volmat
2025-01-08 13:16 ` [PATCH v2 7/8] media: stm32: csi: correct unsigned or useless variable settings Alain Volmat
2025-01-08 13:16 ` [PATCH v2 8/8] media: stm32: dcmipp: add has_csi2 & needs_mclk in match data Alain Volmat
2025-01-10  8:08 ` [PATCH v2 0/8] media: stm32: csi / dcmipp corrections Sakari Ailus
2025-01-10  8:10   ` Sakari Ailus
2025-01-13  8:55   ` Alain Volmat

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

all inboxes | Powered by JetHome®