All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2
@ 2026-01-16 16:17 Frank Li
  2026-01-16 16:17 ` [PATCH RESEND v2 1/5] media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment Frank Li
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Frank Li @ 2026-01-16 16:17 UTC (permalink / raw)
  To: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: imx, linux-media, linux-staging, linux-arm-kernel, linux-kernel,
	Frank Li, Laurent Pinchart

Previous https://lore.kernel.org/imx/20250821-95_cam-v3-0-c9286fbb34b9@nxp.com/
There are too much patches (32) in above thread.

Just extract first 6 cleanup patches to review easily. The overall road
map see above 32 patch serise.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v2:
- collect review by tags
- drop media: staging: media: imx6-mipi-csi2: use devm_add_action_or_reset() to simplify code
  need more time to understand nf_unregister() and subdev_unregister()'s relationship.
- detail change see each patch's change log
- move devm_mutex_init() patch to second one
- Link to v1: https://lore.kernel.org/r/20251107-stage-csi2-cleanup-v1-0-5d42535243ac@nxp.com

---
Frank Li (5):
      media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment
      media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code
      media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks
      media: staging: media: imx6-mipi-csi2: use guard() to simplify code
      media: staging: media: imx6-mipi-csi2: use devm_platform_ioremap_resource() simplify code

 drivers/staging/media/imx/imx6-mipi-csi2.c | 209 +++++++++++------------------
 1 file changed, 80 insertions(+), 129 deletions(-)
---
base-commit: df5d79720b152e7ff058f11ed7e88d5b5c8d2a0c
change-id: 20251106-stage-csi2-cleanup-6db1715fd187

Best regards,
--
Frank Li <Frank.Li@nxp.com>
-- 
Frank Li <Frank.Li@nxp.com>


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

* [PATCH RESEND v2 1/5] media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment
  2026-01-16 16:17 [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Frank Li
@ 2026-01-16 16:17 ` Frank Li
  2026-01-21  1:59   ` Laurent Pinchart
  2026-01-16 16:17 ` [PATCH RESEND v2 2/5] media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code Frank Li
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2026-01-16 16:17 UTC (permalink / raw)
  To: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: imx, linux-media, linux-staging, linux-arm-kernel, linux-kernel,
	Frank Li

Replace spaces with tabs to align register value definitions, making it
easier to add new entries and maintain consistent formatting.

Also use a space between the type and field in struct csi2_dev.

No functional change.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v2
- struct use one space between type and field name.
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 84 +++++++++++++++---------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index dd8c7b3233bccfc34b59e0f0ff813b36752e1526..1113ea2a37f03753423164069b95c049968cc0af 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -23,65 +23,65 @@
  * there must be 5 pads: 1 input pad from sensor, and
  * the 4 virtual channel output pads
  */
-#define CSI2_SINK_PAD       0
-#define CSI2_NUM_SINK_PADS  1
-#define CSI2_NUM_SRC_PADS   4
-#define CSI2_NUM_PADS       5
+#define CSI2_SINK_PAD		0
+#define CSI2_NUM_SINK_PADS	1
+#define CSI2_NUM_SRC_PADS	4
+#define CSI2_NUM_PADS		5
 
 /*
  * The default maximum bit-rate per lane in Mbps, if the
  * source subdev does not provide V4L2_CID_LINK_FREQ.
  */
-#define CSI2_DEFAULT_MAX_MBPS 849
+#define CSI2_DEFAULT_MAX_MBPS	849
 
 struct csi2_dev {
-	struct device          *dev;
-	struct v4l2_subdev      sd;
+	struct device *dev;
+	struct v4l2_subdev sd;
 	struct v4l2_async_notifier notifier;
-	struct media_pad       pad[CSI2_NUM_PADS];
-	struct clk             *dphy_clk;
-	struct clk             *pllref_clk;
-	struct clk             *pix_clk; /* what is this? */
-	void __iomem           *base;
+	struct media_pad pad[CSI2_NUM_PADS];
+	struct clk *dphy_clk;
+	struct clk *pllref_clk;
+	struct clk *pix_clk; /* what is this? */
+	void __iomem *base;
 
-	struct v4l2_subdev	*remote;
-	unsigned int		remote_pad;
-	unsigned short		data_lanes;
+	struct v4l2_subdev *remote;
+	unsigned int remote_pad;
+	unsigned short data_lanes;
 
 	/* lock to protect all members below */
 	struct mutex lock;
 
 	struct v4l2_mbus_framefmt format_mbus;
 
-	int                     stream_count;
-	struct v4l2_subdev      *src_sd;
-	bool                    sink_linked[CSI2_NUM_SRC_PADS];
+	int stream_count;
+	struct v4l2_subdev *src_sd;
+	bool sink_linked[CSI2_NUM_SRC_PADS];
 };
 
 #define DEVICE_NAME "imx6-mipi-csi2"
 
 /* Register offsets */
-#define CSI2_VERSION            0x000
-#define CSI2_N_LANES            0x004
-#define CSI2_PHY_SHUTDOWNZ      0x008
-#define CSI2_DPHY_RSTZ          0x00c
-#define CSI2_RESETN             0x010
-#define CSI2_PHY_STATE          0x014
-#define PHY_STOPSTATEDATA_BIT   4
-#define PHY_STOPSTATEDATA(n)    BIT(PHY_STOPSTATEDATA_BIT + (n))
-#define PHY_RXCLKACTIVEHS       BIT(8)
-#define PHY_RXULPSCLKNOT        BIT(9)
-#define PHY_STOPSTATECLK        BIT(10)
-#define CSI2_DATA_IDS_1         0x018
-#define CSI2_DATA_IDS_2         0x01c
-#define CSI2_ERR1               0x020
-#define CSI2_ERR2               0x024
-#define CSI2_MSK1               0x028
-#define CSI2_MSK2               0x02c
-#define CSI2_PHY_TST_CTRL0      0x030
+#define CSI2_VERSION		0x000
+#define CSI2_N_LANES		0x004
+#define CSI2_PHY_SHUTDOWNZ	0x008
+#define CSI2_DPHY_RSTZ		0x00c
+#define CSI2_RESETN		0x010
+#define CSI2_PHY_STATE		0x014
+#define PHY_STOPSTATEDATA_BIT	4
+#define PHY_STOPSTATEDATA(n)	BIT(PHY_STOPSTATEDATA_BIT + (n))
+#define PHY_RXCLKACTIVEHS	BIT(8)
+#define PHY_RXULPSCLKNOT	BIT(9)
+#define PHY_STOPSTATECLK	BIT(10)
+#define CSI2_DATA_IDS_1		0x018
+#define CSI2_DATA_IDS_2		0x01c
+#define CSI2_ERR1		0x020
+#define CSI2_ERR2		0x024
+#define CSI2_MSK1		0x028
+#define CSI2_MSK2		0x02c
+#define CSI2_PHY_TST_CTRL0	0x030
 #define PHY_TESTCLR		BIT(0)
 #define PHY_TESTCLK		BIT(1)
-#define CSI2_PHY_TST_CTRL1      0x034
+#define CSI2_PHY_TST_CTRL1	0x034
 #define PHY_TESTEN		BIT(16)
 /*
  * i.MX CSI2IPU Gasket registers follow. The CSI2IPU gasket is
@@ -106,13 +106,13 @@ static inline struct csi2_dev *notifier_to_dev(struct v4l2_async_notifier *n)
  * reference manual is as follows:
  *
  * 1. Deassert presetn signal (global reset).
- *        It's not clear what this "global reset" signal is (maybe APB
- *        global reset), but in any case this step would be probably
- *        be carried out during driver load in csi2_probe().
+ *	It's not clear what this "global reset" signal is (maybe APB
+ *	global reset), but in any case this step would be probably
+ *	be carried out during driver load in csi2_probe().
  *
  * 2. Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state.
- *        This must be carried out by the MIPI sensor's s_power(ON) subdev
- *        op.
+ *	This must be carried out by the MIPI sensor's s_power(ON) subdev
+ *	op.
  *
  * 3. D-PHY initialization.
  * 4. CSI2 Controller programming (Set N_LANES, deassert PHY_SHUTDOWNZ,

-- 
2.34.1


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

* [PATCH RESEND v2 2/5] media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code
  2026-01-16 16:17 [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Frank Li
  2026-01-16 16:17 ` [PATCH RESEND v2 1/5] media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment Frank Li
@ 2026-01-16 16:17 ` Frank Li
  2026-01-21  2:00   ` Laurent Pinchart
  2026-01-16 16:17 ` [PATCH RESEND v2 3/5] media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks Frank Li
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2026-01-16 16:17 UTC (permalink / raw)
  To: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: imx, linux-media, linux-staging, linux-arm-kernel, linux-kernel,
	Frank Li

Use devm_mutex_init() to simplify the code. No functional change.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 1113ea2a37f03753423164069b95c049968cc0af..4f740170d2bbf586ac0a58b5d25f8f8432e9e6a3 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -777,12 +777,14 @@ static int csi2_probe(struct platform_device *pdev)
 	if (!csi2->base)
 		return -ENOMEM;
 
-	mutex_init(&csi2->lock);
+	ret = devm_mutex_init(&pdev->dev, &csi2->lock);
+	if (ret)
+		return ret;
 
 	ret = clk_prepare_enable(csi2->pllref_clk);
 	if (ret) {
 		v4l2_err(&csi2->sd, "failed to enable pllref_clk\n");
-		goto rmmutex;
+		return ret;
 	}
 
 	ret = clk_prepare_enable(csi2->dphy_clk);
@@ -805,8 +807,6 @@ static int csi2_probe(struct platform_device *pdev)
 	clk_disable_unprepare(csi2->dphy_clk);
 pllref_off:
 	clk_disable_unprepare(csi2->pllref_clk);
-rmmutex:
-	mutex_destroy(&csi2->lock);
 	return ret;
 }
 
@@ -820,7 +820,6 @@ static void csi2_remove(struct platform_device *pdev)
 	v4l2_async_unregister_subdev(sd);
 	clk_disable_unprepare(csi2->dphy_clk);
 	clk_disable_unprepare(csi2->pllref_clk);
-	mutex_destroy(&csi2->lock);
 	media_entity_cleanup(&sd->entity);
 }
 

-- 
2.34.1


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

* [PATCH RESEND v2 3/5] media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks
  2026-01-16 16:17 [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Frank Li
  2026-01-16 16:17 ` [PATCH RESEND v2 1/5] media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment Frank Li
  2026-01-16 16:17 ` [PATCH RESEND v2 2/5] media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code Frank Li
@ 2026-01-16 16:17 ` Frank Li
  2026-01-21  2:08   ` Laurent Pinchart
  2026-01-16 16:17 ` [PATCH RESEND v2 4/5] media: staging: media: imx6-mipi-csi2: use guard() to simplify code Frank Li
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2026-01-16 16:17 UTC (permalink / raw)
  To: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: imx, linux-media, linux-staging, linux-arm-kernel, linux-kernel,
	Frank Li

Use devm_clk_bulk_get_all_enabled() helper to simplify clock handling.

Defer all clock prepare and enable to csi2_start(), which previous only
enable pix clock here.

Add clk_enable at log_status().

Do that safely because there are not register access before csi2_start().

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v2
- add clk_bulk_prepare_enable() get at csi2_log_status()
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 57 +++++++++---------------------
 1 file changed, 16 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 4f740170d2bbf586ac0a58b5d25f8f8432e9e6a3..e1b4b7fb53131ce9515b9441d8fc420e85d3e993 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -39,9 +39,8 @@ struct csi2_dev {
 	struct v4l2_subdev sd;
 	struct v4l2_async_notifier notifier;
 	struct media_pad pad[CSI2_NUM_PADS];
-	struct clk *dphy_clk;
-	struct clk *pllref_clk;
-	struct clk *pix_clk; /* what is this? */
+	struct clk_bulk_data *clks;
+	int num_clks;
 	void __iomem *base;
 
 	struct v4l2_subdev *remote;
@@ -343,7 +342,7 @@ static int csi2_start(struct csi2_dev *csi2)
 	unsigned int lanes;
 	int ret;
 
-	ret = clk_prepare_enable(csi2->pix_clk);
+	ret = clk_bulk_prepare_enable(csi2->num_clks, csi2->clks);
 	if (ret)
 		return ret;
 
@@ -390,7 +389,7 @@ static int csi2_start(struct csi2_dev *csi2)
 err_assert_reset:
 	csi2_enable(csi2, false);
 err_disable_clk:
-	clk_disable_unprepare(csi2->pix_clk);
+	clk_bulk_disable_unprepare(csi2->num_clks, csi2->clks);
 	return ret;
 }
 
@@ -401,7 +400,7 @@ static void csi2_stop(struct csi2_dev *csi2)
 	v4l2_subdev_call(csi2->src_sd, video, post_streamoff);
 
 	csi2_enable(csi2, false);
-	clk_disable_unprepare(csi2->pix_clk);
+	clk_bulk_disable_unprepare(csi2->num_clks, csi2->clks);
 }
 
 /*
@@ -570,6 +569,11 @@ static int csi2_registered(struct v4l2_subdev *sd)
 static int csi2_log_status(struct v4l2_subdev *sd)
 {
 	struct csi2_dev *csi2 = sd_to_dev(sd);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(csi2->num_clks, csi2->clks);
+	if (ret)
+		return ret;
 
 	v4l2_info(sd, "-----MIPI CSI status-----\n");
 	v4l2_info(sd, "VERSION: 0x%x\n",
@@ -601,6 +605,8 @@ static int csi2_log_status(struct v4l2_subdev *sd)
 	v4l2_info(sd, "PHY_TST_CTRL1: 0x%x\n",
 		  readl(csi2->base + CSI2_PHY_TST_CTRL1));
 
+	clk_bulk_disable_unprepare(csi2->num_clks, csi2->clks);
+
 	return 0;
 }
 
@@ -749,24 +755,6 @@ static int csi2_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	csi2->pllref_clk = devm_clk_get(&pdev->dev, "ref");
-	if (IS_ERR(csi2->pllref_clk)) {
-		v4l2_err(&csi2->sd, "failed to get pll reference clock\n");
-		return PTR_ERR(csi2->pllref_clk);
-	}
-
-	csi2->dphy_clk = devm_clk_get(&pdev->dev, "dphy");
-	if (IS_ERR(csi2->dphy_clk)) {
-		v4l2_err(&csi2->sd, "failed to get dphy clock\n");
-		return PTR_ERR(csi2->dphy_clk);
-	}
-
-	csi2->pix_clk = devm_clk_get(&pdev->dev, "pix");
-	if (IS_ERR(csi2->pix_clk)) {
-		v4l2_err(&csi2->sd, "failed to get pixel clock\n");
-		return PTR_ERR(csi2->pix_clk);
-	}
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		v4l2_err(&csi2->sd, "failed to get platform resources\n");
@@ -781,20 +769,12 @@ static int csi2_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(csi2->pllref_clk);
-	if (ret) {
-		v4l2_err(&csi2->sd, "failed to enable pllref_clk\n");
-		return ret;
-	}
-
-	ret = clk_prepare_enable(csi2->dphy_clk);
-	if (ret) {
-		v4l2_err(&csi2->sd, "failed to enable dphy_clk\n");
-		goto pllref_off;
-	}
-
 	platform_set_drvdata(pdev, &csi2->sd);
 
+	csi2->num_clks = devm_clk_bulk_get_all(&pdev->dev, &csi2->clks);
+	if (csi2->num_clks < 0)
+		return dev_err_probe(&pdev->dev, csi2->num_clks, "Failed to get clocks\n");
+
 	ret = csi2_async_register(csi2);
 	if (ret)
 		goto clean_notifier;
@@ -804,9 +784,6 @@ static int csi2_probe(struct platform_device *pdev)
 clean_notifier:
 	v4l2_async_nf_unregister(&csi2->notifier);
 	v4l2_async_nf_cleanup(&csi2->notifier);
-	clk_disable_unprepare(csi2->dphy_clk);
-pllref_off:
-	clk_disable_unprepare(csi2->pllref_clk);
 	return ret;
 }
 
@@ -818,8 +795,6 @@ static void csi2_remove(struct platform_device *pdev)
 	v4l2_async_nf_unregister(&csi2->notifier);
 	v4l2_async_nf_cleanup(&csi2->notifier);
 	v4l2_async_unregister_subdev(sd);
-	clk_disable_unprepare(csi2->dphy_clk);
-	clk_disable_unprepare(csi2->pllref_clk);
 	media_entity_cleanup(&sd->entity);
 }
 

-- 
2.34.1


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

* [PATCH RESEND v2 4/5] media: staging: media: imx6-mipi-csi2: use guard() to simplify code
  2026-01-16 16:17 [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Frank Li
                   ` (2 preceding siblings ...)
  2026-01-16 16:17 ` [PATCH RESEND v2 3/5] media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks Frank Li
@ 2026-01-16 16:17 ` Frank Li
  2026-01-21  2:13   ` Laurent Pinchart
  2026-01-16 16:18 ` [PATCH RESEND v2 5/5] media: staging: media: imx6-mipi-csi2: use devm_platform_ioremap_resource() " Frank Li
  2026-01-21  2:17 ` [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Laurent Pinchart
  5 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2026-01-16 16:17 UTC (permalink / raw)
  To: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: imx, linux-media, linux-staging, linux-arm-kernel, linux-kernel,
	Frank Li

Use guard() to simplify mutex locking. No functional change.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
leave as it without cleanup goto branch because there are two path to
update stream_count.

And it will be replaced soon at

Use new v4l2_subdev_pad_ops.enable_streams(disalbe_stream) replace
deprecated s_stream interface.

https://lore.kernel.org/imx/20250821-95_cam-v3-18-c9286fbb34b9@nxp.com/
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 54 +++++++++++-------------------
 1 file changed, 19 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index e1b4b7fb53131ce9515b9441d8fc420e85d3e993..762f19ffd0858c952027afa8e0f36fc87246e1ea 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -412,21 +412,17 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
 	struct csi2_dev *csi2 = sd_to_dev(sd);
 	int i, ret = 0;
 
-	mutex_lock(&csi2->lock);
+	guard(mutex)(&csi2->lock);
 
-	if (!csi2->src_sd) {
-		ret = -EPIPE;
-		goto out;
-	}
+	if (!csi2->src_sd)
+		return -EPIPE;
 
 	for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
 		if (csi2->sink_linked[i])
 			break;
 	}
-	if (i >= CSI2_NUM_SRC_PADS) {
-		ret = -EPIPE;
-		goto out;
-	}
+	if (i >= CSI2_NUM_SRC_PADS)
+		return -EPIPE;
 
 	/*
 	 * enable/disable streaming only if stream_count is
@@ -441,14 +437,12 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
 	else
 		csi2_stop(csi2);
 	if (ret)
-		goto out;
+		return ret;
 
 update_count:
 	csi2->stream_count += enable ? 1 : -1;
 	if (csi2->stream_count < 0)
 		csi2->stream_count = 0;
-out:
-	mutex_unlock(&csi2->lock);
 	return ret;
 }
 
@@ -466,32 +460,28 @@ static int csi2_link_setup(struct media_entity *entity,
 
 	remote_sd = media_entity_to_v4l2_subdev(remote->entity);
 
-	mutex_lock(&csi2->lock);
+	guard(mutex)(&csi2->lock);
 
 	if (local->flags & MEDIA_PAD_FL_SOURCE) {
 		if (flags & MEDIA_LNK_FL_ENABLED) {
-			if (csi2->sink_linked[local->index - 1]) {
-				ret = -EBUSY;
-				goto out;
-			}
+			if (csi2->sink_linked[local->index - 1])
+				return -EBUSY;
+
 			csi2->sink_linked[local->index - 1] = true;
 		} else {
 			csi2->sink_linked[local->index - 1] = false;
 		}
 	} else {
 		if (flags & MEDIA_LNK_FL_ENABLED) {
-			if (csi2->src_sd) {
-				ret = -EBUSY;
-				goto out;
-			}
+			if (csi2->src_sd)
+				return -EBUSY;
+
 			csi2->src_sd = remote_sd;
 		} else {
 			csi2->src_sd = NULL;
 		}
 	}
 
-out:
-	mutex_unlock(&csi2->lock);
 	return ret;
 }
 
@@ -512,14 +502,12 @@ static int csi2_get_fmt(struct v4l2_subdev *sd,
 	struct csi2_dev *csi2 = sd_to_dev(sd);
 	struct v4l2_mbus_framefmt *fmt;
 
-	mutex_lock(&csi2->lock);
+	guard(mutex)(&csi2->lock);
 
 	fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
 
 	sdformat->format = *fmt;
 
-	mutex_unlock(&csi2->lock);
-
 	return 0;
 }
 
@@ -529,17 +517,14 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
 {
 	struct csi2_dev *csi2 = sd_to_dev(sd);
 	struct v4l2_mbus_framefmt *fmt;
-	int ret = 0;
 
 	if (sdformat->pad >= CSI2_NUM_PADS)
 		return -EINVAL;
 
-	mutex_lock(&csi2->lock);
+	guard(mutex)(&csi2->lock);
 
-	if (csi2->stream_count > 0) {
-		ret = -EBUSY;
-		goto out;
-	}
+	if (csi2->stream_count > 0)
+		return -EBUSY;
 
 	/* Output pads mirror active input pad, no limits on input pads */
 	if (sdformat->pad != CSI2_SINK_PAD)
@@ -548,9 +533,8 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
 	fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
 
 	*fmt = sdformat->format;
-out:
-	mutex_unlock(&csi2->lock);
-	return ret;
+
+	return 0;
 }
 
 static int csi2_registered(struct v4l2_subdev *sd)

-- 
2.34.1


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

* [PATCH RESEND v2 5/5] media: staging: media: imx6-mipi-csi2: use devm_platform_ioremap_resource() simplify code
  2026-01-16 16:17 [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Frank Li
                   ` (3 preceding siblings ...)
  2026-01-16 16:17 ` [PATCH RESEND v2 4/5] media: staging: media: imx6-mipi-csi2: use guard() to simplify code Frank Li
@ 2026-01-16 16:18 ` Frank Li
  2026-01-21  2:17 ` [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Laurent Pinchart
  5 siblings, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-01-16 16:18 UTC (permalink / raw)
  To: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: imx, linux-media, linux-staging, linux-arm-kernel, linux-kernel,
	Frank Li, Laurent Pinchart

Use devm_platform_ioremap_resource() simplify code. No functional change.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v4 (reset verson v2 because split to small serise)
- add Laurent Pinchart's review tags
- return PTR_ERR(csi2->base) directly.
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 762f19ffd0858c952027afa8e0f36fc87246e1ea..3d8995dcc9132c1b92c36a65e55476e3ca2703ac 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -709,7 +709,6 @@ static int csi2_async_register(struct csi2_dev *csi2)
 static int csi2_probe(struct platform_device *pdev)
 {
 	struct csi2_dev *csi2;
-	struct resource *res;
 	int i, ret;
 
 	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
@@ -739,15 +738,9 @@ static int csi2_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		v4l2_err(&csi2->sd, "failed to get platform resources\n");
-		return -ENODEV;
-	}
-
-	csi2->base = devm_ioremap(&pdev->dev, res->start, PAGE_SIZE);
-	if (!csi2->base)
-		return -ENOMEM;
+	csi2->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(csi2->base))
+		return PTR_ERR(csi2->base);
 
 	ret = devm_mutex_init(&pdev->dev, &csi2->lock);
 	if (ret)

-- 
2.34.1


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

* Re: [PATCH RESEND v2 1/5] media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment
  2026-01-16 16:17 ` [PATCH RESEND v2 1/5] media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment Frank Li
@ 2026-01-21  1:59   ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2026-01-21  1:59 UTC (permalink / raw)
  To: Frank Li
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, imx, linux-media,
	linux-staging, linux-arm-kernel, linux-kernel

Hi Frank,

Thank you for the patch.

On Fri, Jan 16, 2026 at 11:17:56AM -0500, Frank Li wrote:
> Replace spaces with tabs to align register value definitions, making it
> easier to add new entries and maintain consistent formatting.
> 
> Also use a space between the type and field in struct csi2_dev.
> 
> No functional change.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> changes in v2
> - struct use one space between type and field name.
> ---
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 84 +++++++++++++++---------------
>  1 file changed, 42 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index dd8c7b3233bccfc34b59e0f0ff813b36752e1526..1113ea2a37f03753423164069b95c049968cc0af 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -23,65 +23,65 @@
>   * there must be 5 pads: 1 input pad from sensor, and
>   * the 4 virtual channel output pads
>   */
> -#define CSI2_SINK_PAD       0
> -#define CSI2_NUM_SINK_PADS  1
> -#define CSI2_NUM_SRC_PADS   4
> -#define CSI2_NUM_PADS       5
> +#define CSI2_SINK_PAD		0
> +#define CSI2_NUM_SINK_PADS	1
> +#define CSI2_NUM_SRC_PADS	4
> +#define CSI2_NUM_PADS		5
>  
>  /*
>   * The default maximum bit-rate per lane in Mbps, if the
>   * source subdev does not provide V4L2_CID_LINK_FREQ.
>   */
> -#define CSI2_DEFAULT_MAX_MBPS 849
> +#define CSI2_DEFAULT_MAX_MBPS	849
>  
>  struct csi2_dev {
> -	struct device          *dev;
> -	struct v4l2_subdev      sd;
> +	struct device *dev;
> +	struct v4l2_subdev sd;
>  	struct v4l2_async_notifier notifier;
> -	struct media_pad       pad[CSI2_NUM_PADS];
> -	struct clk             *dphy_clk;
> -	struct clk             *pllref_clk;
> -	struct clk             *pix_clk; /* what is this? */
> -	void __iomem           *base;
> +	struct media_pad pad[CSI2_NUM_PADS];
> +	struct clk *dphy_clk;
> +	struct clk *pllref_clk;
> +	struct clk *pix_clk; /* what is this? */
> +	void __iomem *base;
>  
> -	struct v4l2_subdev	*remote;
> -	unsigned int		remote_pad;
> -	unsigned short		data_lanes;
> +	struct v4l2_subdev *remote;
> +	unsigned int remote_pad;
> +	unsigned short data_lanes;
>  
>  	/* lock to protect all members below */
>  	struct mutex lock;
>  
>  	struct v4l2_mbus_framefmt format_mbus;
>  
> -	int                     stream_count;
> -	struct v4l2_subdev      *src_sd;
> -	bool                    sink_linked[CSI2_NUM_SRC_PADS];
> +	int stream_count;
> +	struct v4l2_subdev *src_sd;
> +	bool sink_linked[CSI2_NUM_SRC_PADS];
>  };
>  
>  #define DEVICE_NAME "imx6-mipi-csi2"
>  
>  /* Register offsets */
> -#define CSI2_VERSION            0x000
> -#define CSI2_N_LANES            0x004
> -#define CSI2_PHY_SHUTDOWNZ      0x008
> -#define CSI2_DPHY_RSTZ          0x00c
> -#define CSI2_RESETN             0x010
> -#define CSI2_PHY_STATE          0x014
> -#define PHY_STOPSTATEDATA_BIT   4
> -#define PHY_STOPSTATEDATA(n)    BIT(PHY_STOPSTATEDATA_BIT + (n))
> -#define PHY_RXCLKACTIVEHS       BIT(8)
> -#define PHY_RXULPSCLKNOT        BIT(9)
> -#define PHY_STOPSTATECLK        BIT(10)
> -#define CSI2_DATA_IDS_1         0x018
> -#define CSI2_DATA_IDS_2         0x01c
> -#define CSI2_ERR1               0x020
> -#define CSI2_ERR2               0x024
> -#define CSI2_MSK1               0x028
> -#define CSI2_MSK2               0x02c
> -#define CSI2_PHY_TST_CTRL0      0x030
> +#define CSI2_VERSION		0x000
> +#define CSI2_N_LANES		0x004
> +#define CSI2_PHY_SHUTDOWNZ	0x008
> +#define CSI2_DPHY_RSTZ		0x00c
> +#define CSI2_RESETN		0x010
> +#define CSI2_PHY_STATE		0x014
> +#define PHY_STOPSTATEDATA_BIT	4
> +#define PHY_STOPSTATEDATA(n)	BIT(PHY_STOPSTATEDATA_BIT + (n))
> +#define PHY_RXCLKACTIVEHS	BIT(8)
> +#define PHY_RXULPSCLKNOT	BIT(9)
> +#define PHY_STOPSTATECLK	BIT(10)
> +#define CSI2_DATA_IDS_1		0x018
> +#define CSI2_DATA_IDS_2		0x01c
> +#define CSI2_ERR1		0x020
> +#define CSI2_ERR2		0x024
> +#define CSI2_MSK1		0x028
> +#define CSI2_MSK2		0x02c
> +#define CSI2_PHY_TST_CTRL0	0x030
>  #define PHY_TESTCLR		BIT(0)
>  #define PHY_TESTCLK		BIT(1)
> -#define CSI2_PHY_TST_CTRL1      0x034
> +#define CSI2_PHY_TST_CTRL1	0x034
>  #define PHY_TESTEN		BIT(16)
>  /*
>   * i.MX CSI2IPU Gasket registers follow. The CSI2IPU gasket is
> @@ -106,13 +106,13 @@ static inline struct csi2_dev *notifier_to_dev(struct v4l2_async_notifier *n)
>   * reference manual is as follows:
>   *
>   * 1. Deassert presetn signal (global reset).
> - *        It's not clear what this "global reset" signal is (maybe APB
> - *        global reset), but in any case this step would be probably
> - *        be carried out during driver load in csi2_probe().
> + *	It's not clear what this "global reset" signal is (maybe APB
> + *	global reset), but in any case this step would be probably
> + *	be carried out during driver load in csi2_probe().
>   *
>   * 2. Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state.
> - *        This must be carried out by the MIPI sensor's s_power(ON) subdev
> - *        op.
> + *	This must be carried out by the MIPI sensor's s_power(ON) subdev
> + *	op.
>   *
>   * 3. D-PHY initialization.
>   * 4. CSI2 Controller programming (Set N_LANES, deassert PHY_SHUTDOWNZ,

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH RESEND v2 2/5] media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code
  2026-01-16 16:17 ` [PATCH RESEND v2 2/5] media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code Frank Li
@ 2026-01-21  2:00   ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2026-01-21  2:00 UTC (permalink / raw)
  To: Frank Li
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, imx, linux-media,
	linux-staging, linux-arm-kernel, linux-kernel

On Fri, Jan 16, 2026 at 11:17:57AM -0500, Frank Li wrote:
> Use devm_mutex_init() to simplify the code. No functional change.
> 
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index 1113ea2a37f03753423164069b95c049968cc0af..4f740170d2bbf586ac0a58b5d25f8f8432e9e6a3 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -777,12 +777,14 @@ static int csi2_probe(struct platform_device *pdev)
>  	if (!csi2->base)
>  		return -ENOMEM;
>  
> -	mutex_init(&csi2->lock);
> +	ret = devm_mutex_init(&pdev->dev, &csi2->lock);
> +	if (ret)
> +		return ret;
>  
>  	ret = clk_prepare_enable(csi2->pllref_clk);
>  	if (ret) {
>  		v4l2_err(&csi2->sd, "failed to enable pllref_clk\n");
> -		goto rmmutex;
> +		return ret;
>  	}
>  
>  	ret = clk_prepare_enable(csi2->dphy_clk);
> @@ -805,8 +807,6 @@ static int csi2_probe(struct platform_device *pdev)
>  	clk_disable_unprepare(csi2->dphy_clk);
>  pllref_off:
>  	clk_disable_unprepare(csi2->pllref_clk);
> -rmmutex:
> -	mutex_destroy(&csi2->lock);
>  	return ret;
>  }
>  
> @@ -820,7 +820,6 @@ static void csi2_remove(struct platform_device *pdev)
>  	v4l2_async_unregister_subdev(sd);
>  	clk_disable_unprepare(csi2->dphy_clk);
>  	clk_disable_unprepare(csi2->pllref_clk);
> -	mutex_destroy(&csi2->lock);
>  	media_entity_cleanup(&sd->entity);
>  }
>  

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH RESEND v2 3/5] media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks
  2026-01-16 16:17 ` [PATCH RESEND v2 3/5] media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks Frank Li
@ 2026-01-21  2:08   ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2026-01-21  2:08 UTC (permalink / raw)
  To: Frank Li
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, imx, linux-media,
	linux-staging, linux-arm-kernel, linux-kernel

On Fri, Jan 16, 2026 at 11:17:58AM -0500, Frank Li wrote:
> Use devm_clk_bulk_get_all_enabled() helper to simplify clock handling.
> 
> Defer all clock prepare and enable to csi2_start(), which previous only
> enable pix clock here.
> 
> Add clk_enable at log_status().
> 
> Do that safely because there are not register access before csi2_start().
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> changes in v2
> - add clk_bulk_prepare_enable() get at csi2_log_status()
> ---
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 57 +++++++++---------------------
>  1 file changed, 16 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index 4f740170d2bbf586ac0a58b5d25f8f8432e9e6a3..e1b4b7fb53131ce9515b9441d8fc420e85d3e993 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -39,9 +39,8 @@ struct csi2_dev {
>  	struct v4l2_subdev sd;
>  	struct v4l2_async_notifier notifier;
>  	struct media_pad pad[CSI2_NUM_PADS];
> -	struct clk *dphy_clk;
> -	struct clk *pllref_clk;
> -	struct clk *pix_clk; /* what is this? */
> +	struct clk_bulk_data *clks;
> +	int num_clks;
>  	void __iomem *base;
>  
>  	struct v4l2_subdev *remote;
> @@ -343,7 +342,7 @@ static int csi2_start(struct csi2_dev *csi2)
>  	unsigned int lanes;
>  	int ret;
>  
> -	ret = clk_prepare_enable(csi2->pix_clk);
> +	ret = clk_bulk_prepare_enable(csi2->num_clks, csi2->clks);
>  	if (ret)
>  		return ret;
>  
> @@ -390,7 +389,7 @@ static int csi2_start(struct csi2_dev *csi2)
>  err_assert_reset:
>  	csi2_enable(csi2, false);
>  err_disable_clk:
> -	clk_disable_unprepare(csi2->pix_clk);
> +	clk_bulk_disable_unprepare(csi2->num_clks, csi2->clks);
>  	return ret;
>  }
>  
> @@ -401,7 +400,7 @@ static void csi2_stop(struct csi2_dev *csi2)
>  	v4l2_subdev_call(csi2->src_sd, video, post_streamoff);
>  
>  	csi2_enable(csi2, false);
> -	clk_disable_unprepare(csi2->pix_clk);
> +	clk_bulk_disable_unprepare(csi2->num_clks, csi2->clks);
>  }
>  
>  /*
> @@ -570,6 +569,11 @@ static int csi2_registered(struct v4l2_subdev *sd)
>  static int csi2_log_status(struct v4l2_subdev *sd)
>  {
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(csi2->num_clks, csi2->clks);
> +	if (ret)
> +		return ret;
>  
>  	v4l2_info(sd, "-----MIPI CSI status-----\n");
>  	v4l2_info(sd, "VERSION: 0x%x\n",
> @@ -601,6 +605,8 @@ static int csi2_log_status(struct v4l2_subdev *sd)
>  	v4l2_info(sd, "PHY_TST_CTRL1: 0x%x\n",
>  		  readl(csi2->base + CSI2_PHY_TST_CTRL1));
>  
> +	clk_bulk_disable_unprepare(csi2->num_clks, csi2->clks);
> +
>  	return 0;
>  }
>  
> @@ -749,24 +755,6 @@ static int csi2_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	csi2->pllref_clk = devm_clk_get(&pdev->dev, "ref");
> -	if (IS_ERR(csi2->pllref_clk)) {
> -		v4l2_err(&csi2->sd, "failed to get pll reference clock\n");
> -		return PTR_ERR(csi2->pllref_clk);
> -	}
> -
> -	csi2->dphy_clk = devm_clk_get(&pdev->dev, "dphy");
> -	if (IS_ERR(csi2->dphy_clk)) {
> -		v4l2_err(&csi2->sd, "failed to get dphy clock\n");
> -		return PTR_ERR(csi2->dphy_clk);
> -	}
> -
> -	csi2->pix_clk = devm_clk_get(&pdev->dev, "pix");
> -	if (IS_ERR(csi2->pix_clk)) {
> -		v4l2_err(&csi2->sd, "failed to get pixel clock\n");
> -		return PTR_ERR(csi2->pix_clk);
> -	}
> -
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
>  		v4l2_err(&csi2->sd, "failed to get platform resources\n");
> @@ -781,20 +769,12 @@ static int csi2_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	ret = clk_prepare_enable(csi2->pllref_clk);
> -	if (ret) {
> -		v4l2_err(&csi2->sd, "failed to enable pllref_clk\n");
> -		return ret;
> -	}
> -
> -	ret = clk_prepare_enable(csi2->dphy_clk);
> -	if (ret) {
> -		v4l2_err(&csi2->sd, "failed to enable dphy_clk\n");
> -		goto pllref_off;
> -	}
> -
>  	platform_set_drvdata(pdev, &csi2->sd);
>  
> +	csi2->num_clks = devm_clk_bulk_get_all(&pdev->dev, &csi2->clks);
> +	if (csi2->num_clks < 0)
> +		return dev_err_probe(&pdev->dev, csi2->num_clks, "Failed to get clocks\n");

I'm still really not a fan of devm_clk_bulk_get_all(). I would prefer
using clk_bulk_get(). The rest looks fine, although I would have split
this patch in two, one to switch to the bulk API, and one to move
enabling/disabling of the clocks.

Ah, no, there's one clock that's already enabled at start time, so a
single patch is fine.

> +
>  	ret = csi2_async_register(csi2);
>  	if (ret)
>  		goto clean_notifier;
> @@ -804,9 +784,6 @@ static int csi2_probe(struct platform_device *pdev)
>  clean_notifier:
>  	v4l2_async_nf_unregister(&csi2->notifier);
>  	v4l2_async_nf_cleanup(&csi2->notifier);
> -	clk_disable_unprepare(csi2->dphy_clk);
> -pllref_off:
> -	clk_disable_unprepare(csi2->pllref_clk);
>  	return ret;
>  }
>  
> @@ -818,8 +795,6 @@ static void csi2_remove(struct platform_device *pdev)
>  	v4l2_async_nf_unregister(&csi2->notifier);
>  	v4l2_async_nf_cleanup(&csi2->notifier);
>  	v4l2_async_unregister_subdev(sd);
> -	clk_disable_unprepare(csi2->dphy_clk);
> -	clk_disable_unprepare(csi2->pllref_clk);
>  	media_entity_cleanup(&sd->entity);
>  }
>  

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH RESEND v2 4/5] media: staging: media: imx6-mipi-csi2: use guard() to simplify code
  2026-01-16 16:17 ` [PATCH RESEND v2 4/5] media: staging: media: imx6-mipi-csi2: use guard() to simplify code Frank Li
@ 2026-01-21  2:13   ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2026-01-21  2:13 UTC (permalink / raw)
  To: Frank Li
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, imx, linux-media,
	linux-staging, linux-arm-kernel, linux-kernel

Hi Frank,

Thank you for the patch.

On Fri, Jan 16, 2026 at 11:17:59AM -0500, Frank Li wrote:
> Use guard() to simplify mutex locking. No functional change.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> leave as it without cleanup goto branch because there are two path to
> update stream_count.
> 
> And it will be replaced soon at
> 
> Use new v4l2_subdev_pad_ops.enable_streams(disalbe_stream) replace
> deprecated s_stream interface.
> 
> https://lore.kernel.org/imx/20250821-95_cam-v3-18-c9286fbb34b9@nxp.com/
> ---
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 54 +++++++++++-------------------
>  1 file changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index e1b4b7fb53131ce9515b9441d8fc420e85d3e993..762f19ffd0858c952027afa8e0f36fc87246e1ea 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -412,21 +412,17 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
>  	int i, ret = 0;
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
> -	if (!csi2->src_sd) {
> -		ret = -EPIPE;
> -		goto out;
> -	}
> +	if (!csi2->src_sd)
> +		return -EPIPE;
>  
>  	for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
>  		if (csi2->sink_linked[i])
>  			break;
>  	}
> -	if (i >= CSI2_NUM_SRC_PADS) {
> -		ret = -EPIPE;
> -		goto out;
> -	}
> +	if (i >= CSI2_NUM_SRC_PADS)
> +		return -EPIPE;
>  
>  	/*
>  	 * enable/disable streaming only if stream_count is
> @@ -441,14 +437,12 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
>  	else
>  		csi2_stop(csi2);
>  	if (ret)
> -		goto out;
> +		return ret;
>  
>  update_count:
>  	csi2->stream_count += enable ? 1 : -1;
>  	if (csi2->stream_count < 0)
>  		csi2->stream_count = 0;
> -out:
> -	mutex_unlock(&csi2->lock);
>  	return ret;
>  }
>  
> @@ -466,32 +460,28 @@ static int csi2_link_setup(struct media_entity *entity,
>  
>  	remote_sd = media_entity_to_v4l2_subdev(remote->entity);
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
>  	if (local->flags & MEDIA_PAD_FL_SOURCE) {
>  		if (flags & MEDIA_LNK_FL_ENABLED) {
> -			if (csi2->sink_linked[local->index - 1]) {
> -				ret = -EBUSY;
> -				goto out;
> -			}
> +			if (csi2->sink_linked[local->index - 1])
> +				return -EBUSY;
> +
>  			csi2->sink_linked[local->index - 1] = true;
>  		} else {
>  			csi2->sink_linked[local->index - 1] = false;
>  		}
>  	} else {
>  		if (flags & MEDIA_LNK_FL_ENABLED) {
> -			if (csi2->src_sd) {
> -				ret = -EBUSY;
> -				goto out;
> -			}
> +			if (csi2->src_sd)
> +				return -EBUSY;
> +
>  			csi2->src_sd = remote_sd;
>  		} else {
>  			csi2->src_sd = NULL;
>  		}
>  	}
>  
> -out:
> -	mutex_unlock(&csi2->lock);
>  	return ret;

You can

	return 0;

here and drop the local ret variable. With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  }
>  
> @@ -512,14 +502,12 @@ static int csi2_get_fmt(struct v4l2_subdev *sd,
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
>  	struct v4l2_mbus_framefmt *fmt;
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
>  	fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
>  
>  	sdformat->format = *fmt;
>  
> -	mutex_unlock(&csi2->lock);
> -
>  	return 0;
>  }
>  
> @@ -529,17 +517,14 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
>  {
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
>  	struct v4l2_mbus_framefmt *fmt;
> -	int ret = 0;
>  
>  	if (sdformat->pad >= CSI2_NUM_PADS)
>  		return -EINVAL;
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
> -	if (csi2->stream_count > 0) {
> -		ret = -EBUSY;
> -		goto out;
> -	}
> +	if (csi2->stream_count > 0)
> +		return -EBUSY;
>  
>  	/* Output pads mirror active input pad, no limits on input pads */
>  	if (sdformat->pad != CSI2_SINK_PAD)
> @@ -548,9 +533,8 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
>  	fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
>  
>  	*fmt = sdformat->format;
> -out:
> -	mutex_unlock(&csi2->lock);
> -	return ret;
> +
> +	return 0;
>  }
>  
>  static int csi2_registered(struct v4l2_subdev *sd)

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2
  2026-01-16 16:17 [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Frank Li
                   ` (4 preceding siblings ...)
  2026-01-16 16:18 ` [PATCH RESEND v2 5/5] media: staging: media: imx6-mipi-csi2: use devm_platform_ioremap_resource() " Frank Li
@ 2026-01-21  2:17 ` Laurent Pinchart
  5 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2026-01-21  2:17 UTC (permalink / raw)
  To: Frank Li
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, imx, linux-media,
	linux-staging, linux-arm-kernel, linux-kernel

On Fri, Jan 16, 2026 at 11:17:55AM -0500, Frank Li wrote:
> Previous https://lore.kernel.org/imx/20250821-95_cam-v3-0-c9286fbb34b9@nxp.com/
> There are too much patches (32) in above thread.
> 
> Just extract first 6 cleanup patches to review easily. The overall road
> map see above 32 patch serise.

I've applied 1/5, 2/5 and 5/5 to my tree.

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Changes in v2:
> - collect review by tags
> - drop media: staging: media: imx6-mipi-csi2: use devm_add_action_or_reset() to simplify code
>   need more time to understand nf_unregister() and subdev_unregister()'s relationship.
> - detail change see each patch's change log
> - move devm_mutex_init() patch to second one
> - Link to v1: https://lore.kernel.org/r/20251107-stage-csi2-cleanup-v1-0-5d42535243ac@nxp.com
> 
> ---
> Frank Li (5):
>       media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment
>       media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code
>       media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks
>       media: staging: media: imx6-mipi-csi2: use guard() to simplify code
>       media: staging: media: imx6-mipi-csi2: use devm_platform_ioremap_resource() simplify code
> 
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 209 +++++++++++------------------
>  1 file changed, 80 insertions(+), 129 deletions(-)
> ---
> base-commit: df5d79720b152e7ff058f11ed7e88d5b5c8d2a0c
> change-id: 20251106-stage-csi2-cleanup-6db1715fd187

-- 
Regards,

Laurent Pinchart

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

end of thread, other threads:[~2026-01-21  2:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-16 16:17 [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Frank Li
2026-01-16 16:17 ` [PATCH RESEND v2 1/5] media: staging: media: imx6-mipi-csi2: replace spaces with tabs for alignment Frank Li
2026-01-21  1:59   ` Laurent Pinchart
2026-01-16 16:17 ` [PATCH RESEND v2 2/5] media: staging: media: imx6-mipi-csi2: use devm_mutex_init() to simplify code Frank Li
2026-01-21  2:00   ` Laurent Pinchart
2026-01-16 16:17 ` [PATCH RESEND v2 3/5] media: staging: media: imx6-mipi-csi2: use devm_clk_bulk_get_all() to fetch clocks Frank Li
2026-01-21  2:08   ` Laurent Pinchart
2026-01-16 16:17 ` [PATCH RESEND v2 4/5] media: staging: media: imx6-mipi-csi2: use guard() to simplify code Frank Li
2026-01-21  2:13   ` Laurent Pinchart
2026-01-16 16:18 ` [PATCH RESEND v2 5/5] media: staging: media: imx6-mipi-csi2: use devm_platform_ioremap_resource() " Frank Li
2026-01-21  2:17 ` [PATCH RESEND v2 0/5] media: staging: media: imx6-mipi-csi2: trivial cleanup to prepare convert to common dw mipi csi2 Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.