[PATCH v7 0/5] Add MIPI CSI-2 support for i.MX8ULP

[PATCH v7 0/5] Add MIPI CSI-2 support for i.MX8ULP

[Guoniu Zhou]

The serial adds MIPI CSI-2 support for i.MX8ULP.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v7:
- No functional changes, only move patch 2 after patch 4 to avoid dependency issues.
- Rebased on latest media/next
- Link to v6: https://lore.kernel.org/r/20250917-csi2_imx8ulp-v6-0-23a355982eff@nxp.com

Changes in v6:
- s/existed/existing/ in patch 1 commit log.
- Delete the compatible between 8ulp and 8qxp since 8ulp need one more clock.
- Update compatible string in dts for csi node.
- Add one new patch to handle 8ulp in driver.
    media: imx8mq-mipi-csi2: Add support for i.MX8ULP
- Link to v5: https://lore.kernel.org/r/20250901-csi2_imx8ulp-v5-0-67964d1471f3@nxp.com

Changes in v5:
- Delete else: block and move clock constrains to each case.
- List exact cases, but put imx8qxp/qm in one if:then: block to avoid
  repetitive code since they are same.
- Link to v4: https://lore.kernel.org/all/20250828-csi2_imx8ulp-v4-0-a2f97b15bb98@nxp.com

Changes in v4:
- Change csr clock name to pclk which is more readability.
- Add restriction to i.MX8ULP and the other variants remain the same as previous versions.
- Update commit log in patch 1 to describe why add new compatible string for i.MX8ULP.
- Link to v3: https://lore.kernel.org/all/20250825-csi2_imx8ulp-v3-0-35885aba62bc@nxp.com

Changes in v3:
- Correct the order of "fsl,imx8qm-mipi-csi2","fsl,imx8qm-mipi-csi2".
- Correct the order of minItems and maxItems.
- Restict all variants.
- Change pclk clock name to csr to match IP port name.
- Align description about csr clock with IP datasheet.
- Add reasons for adding a fourth clock(csr) in patch 1 commit log.
- Link to v2: https://lore.kernel.org/all/20250822-csi2_imx8ulp-v2-0-26a444394965@nxp.com

Changes in v2:
- Add more description about pclk clock.
- Change minItems/maxItems to 2 for resets property.
- Better to handle "fsl,imx8ulp-mipi-csi2" variant.
- Move comment to the top of reset_control_deassert().
- Move dts patch as the last one.
- Add "fsl,imx8qxp-mipi-csi2" to compatible string list of csi node.
- Remove patch 5 in v1.
- Link to v1: https://lore.kernel.org/all/20250812081923.1019345-1-guoniu.zhou@oss.nxp.com

---
Guoniu Zhou (5):
      media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string
      media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks
      media: imx8mq-mipi-csi2: Explicitly release reset
      media: imx8mq-mipi-csi2: Add support for i.MX8ULP
      arm64: dts: imx8ulp: Add CSI and ISI Nodes

 .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41 ++++++++++++-
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi         | 67 ++++++++++++++++++++++
 drivers/media/platform/nxp/imx8mq-mipi-csi2.c      | 61 ++++++--------------
 3 files changed, 124 insertions(+), 45 deletions(-)
---
base-commit: 1fdb55ed40fa5ebe6934bd6b93036c714ebb5ef8
change-id: 20250819-csi2_imx8ulp-9db386dd6bdf

Best regards,
-- 
Guoniu Zhou <guoniu.zhou@nxp.com>

[PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

The CSI-2 receiver in the i.MX8ULP is almost identical to the version
present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
clock as the input clock for its APB interface of Control and Status
register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
increase maxItems of Clocks (clock-names) to 4 from 3.  And keep the
same restriction for existing compatible.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41 ++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
index 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f24837443f7f4ab6418 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
@@ -20,6 +20,7 @@ properties:
       - enum:
           - fsl,imx8mq-mipi-csi2
           - fsl,imx8qxp-mipi-csi2
+          - fsl,imx8ulp-mipi-csi2
       - items:
           - const: fsl,imx8qm-mipi-csi2
           - const: fsl,imx8qxp-mipi-csi2
@@ -39,12 +40,16 @@ properties:
                      clock that the RX DPHY receives.
       - description: ui is the pixel clock (phy_ref up to 333Mhz).
                      See the reference manual for details.
+      - description: pclk is clock for csr APB interface.
+    minItems: 3
 
   clock-names:
     items:
       - const: core
       - const: esc
       - const: ui
+      - const: pclk
+    minItems: 3
 
   power-domains:
     maxItems: 1
@@ -130,19 +135,51 @@ allOf:
         compatible:
           contains:
             enum:
-              - fsl,imx8qxp-mipi-csi2
+              - fsl,imx8ulp-mipi-csi2
+    then:
+      properties:
+        reg:
+          minItems: 2
+        resets:
+          minItems: 2
+          maxItems: 2
+        clocks:
+          minItems: 4
+        clock-names:
+          minItems: 4
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,imx8qxp-mipi-csi2
     then:
       properties:
         reg:
           minItems: 2
         resets:
           maxItems: 1
-    else:
+        clocks:
+          maxItems: 3
+        clock-names:
+          maxItems: 3
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx8mq-mipi-csi2
+    then:
       properties:
         reg:
           maxItems: 1
         resets:
           minItems: 3
+        clocks:
+          maxItems: 3
+        clock-names:
+          maxItems: 3
       required:
         - fsl,mipi-phy-gpr
 

-- 
2.34.1

[PATCH v7 2/5] media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

Use devm_clk_bulk_get_all() helper to simplify clock handle code.

No functional changes intended.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 52 ++++++++-------------------
 1 file changed, 15 insertions(+), 37 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
index d333ff43539f061b8b9cf88af2cda8c44b3ec2a9..fd202601d401145da8be23df4451f6af660642c5 100644
--- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
+++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
@@ -71,21 +71,6 @@ enum {
 	ST_SUSPENDED	= 4,
 };
 
-enum imx8mq_mipi_csi_clk {
-	CSI2_CLK_CORE,
-	CSI2_CLK_ESC,
-	CSI2_CLK_UI,
-	CSI2_NUM_CLKS,
-};
-
-static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
-	[CSI2_CLK_CORE] = "core",
-	[CSI2_CLK_ESC] = "esc",
-	[CSI2_CLK_UI] = "ui",
-};
-
-#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
-
 struct imx8mq_plat_data {
 	int (*enable)(struct csi_state *state, u32 hs_settle);
 	void (*disable)(struct csi_state *state);
@@ -111,7 +96,8 @@ struct csi_state {
 	struct device *dev;
 	const struct imx8mq_plat_data *pdata;
 	void __iomem *regs;
-	struct clk_bulk_data clks[CSI2_NUM_CLKS];
+	struct clk_bulk_data *clks;
+	int num_clks;
 	struct reset_control *rst;
 	struct regulator *mipi_phy_regulator;
 
@@ -384,24 +370,16 @@ static void imx8mq_mipi_csi_set_params(struct csi_state *state)
 			      CSI2RX_SEND_LEVEL);
 }
 
-static int imx8mq_mipi_csi_clk_enable(struct csi_state *state)
-{
-	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
-}
-
-static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
+static struct clk *find_esc_clk(struct csi_state *state)
 {
-	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
-}
-
-static int imx8mq_mipi_csi_clk_get(struct csi_state *state)
-{
-	unsigned int i;
+	int i;
 
-	for (i = 0; i < CSI2_NUM_CLKS; i++)
-		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
+	for (i = 0; i < state->num_clks; i++) {
+		if (!strcmp(state->clks[i].id, "esc"))
+			return state->clks[i].clk;
+	}
 
-	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
+	return NULL;
 }
 
 static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
@@ -456,7 +434,7 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
 	 * documentation recommends picking a value away from the boundaries.
 	 * Let's pick the average.
 	 */
-	esc_clk_rate = clk_get_rate(state->clks[CSI2_CLK_ESC].clk);
+	esc_clk_rate = clk_get_rate(find_esc_clk(state));
 	if (!esc_clk_rate) {
 		dev_err(state->dev, "Could not get esc clock rate.\n");
 		return -EINVAL;
@@ -783,7 +761,7 @@ static void imx8mq_mipi_csi_pm_suspend(struct device *dev)
 
 	if (state->state & ST_POWERED) {
 		imx8mq_mipi_csi_stop_stream(state);
-		imx8mq_mipi_csi_clk_disable(state);
+		clk_bulk_disable_unprepare(state->num_clks, state->clks);
 		state->state &= ~ST_POWERED;
 	}
 
@@ -801,7 +779,7 @@ static int imx8mq_mipi_csi_pm_resume(struct device *dev)
 
 	if (!(state->state & ST_POWERED)) {
 		state->state |= ST_POWERED;
-		ret = imx8mq_mipi_csi_clk_enable(state);
+		ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
 	}
 	if (state->state & ST_STREAMING) {
 		sd_state = v4l2_subdev_lock_and_get_active_state(sd);
@@ -1027,9 +1005,9 @@ static int imx8mq_mipi_csi_probe(struct platform_device *pdev)
 	if (IS_ERR(state->regs))
 		return PTR_ERR(state->regs);
 
-	ret = imx8mq_mipi_csi_clk_get(state);
-	if (ret < 0)
-		return ret;
+	state->num_clks = devm_clk_bulk_get_all(dev, &state->clks);
+	if (state->num_clks < 0)
+		return dev_err_probe(dev, state->num_clks, "Failed to get clocks\n");
 
 	platform_set_drvdata(pdev, &state->sd);
 

-- 
2.34.1

[PATCH v7 3/5] media: imx8mq-mipi-csi2: Explicitly release reset

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

Call reset_control_deassert() to explicitly release reset to make sure
reset bits are cleared since platform like i.MX8ULP can't clear reset
bits automatically.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
index fd202601d401145da8be23df4451f6af660642c5..fd788a7f48e5feeff658e3d2347db6fefca5d0cf 100644
--- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
+++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
@@ -337,18 +337,14 @@ static int imx8mq_mipi_csi_sw_reset(struct csi_state *state)
 {
 	int ret;
 
-	/*
-	 * these are most likely self-clearing reset bits. to make it
-	 * more clear, the reset-imx7 driver should implement the
-	 * .reset() operation.
-	 */
 	ret = reset_control_assert(state->rst);
 	if (ret < 0) {
 		dev_err(state->dev, "Failed to assert resets: %d\n", ret);
 		return ret;
 	}
 
-	return 0;
+	/* Explicitly release reset to make sure reset bits are cleared. */
+	return reset_control_deassert(state->rst);
 }
 
 static void imx8mq_mipi_csi_set_params(struct csi_state *state)

-- 
2.34.1

[PATCH v7 4/5] media: imx8mq-mipi-csi2: Add support for i.MX8ULP

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

The CSI-2 receiver in i.MX8ULP is almost same as i.MX8QXP/QM except
clocks and resets, so add compatible string for i.MX8ULP to handle
the difference and reuse platform data of i.MX8QXP/QM.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
index fd788a7f48e5feeff658e3d2347db6fefca5d0cf..d8fadb0f1b6b670110ee98a74cffd56a6c96592b 100644
--- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
+++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
@@ -1069,6 +1069,7 @@ static void imx8mq_mipi_csi_remove(struct platform_device *pdev)
 static const struct of_device_id imx8mq_mipi_csi_of_match[] = {
 	{ .compatible = "fsl,imx8mq-mipi-csi2", .data = &imx8mq_data },
 	{ .compatible = "fsl,imx8qxp-mipi-csi2", .data = &imx8qxp_data },
+	{ .compatible = "fsl,imx8ulp-mipi-csi2", .data = &imx8qxp_data },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, imx8mq_mipi_csi_of_match);

-- 
2.34.1

[PATCH v7 5/5] arm64: dts: imx8ulp: Add CSI and ISI Nodes

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

The CSI-2 in the i.MX8ULP is almost identical to the version present
in the i.MX8QXP/QM and is routed to the ISI. Add both the ISI and CSI
nodes and mark them as disabled by default since capture is dependent
on an attached camera.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 67 ++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 13b01f3aa2a4950c37e72e04f6bfb5995dc19178..99271d55dfb60ed2cbfe664d928be179eb257674 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -7,6 +7,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/power/imx8ulp-power.h>
+#include <dt-bindings/reset/imx8ulp-pcc-reset.h>
 #include <dt-bindings/thermal/thermal.h>
 
 #include "imx8ulp-pinfunc.h"
@@ -842,6 +843,72 @@ spdif: spdif@2dab0000 {
 				dma-names = "rx", "tx";
 				status = "disabled";
 			};
+
+			isi: isi@2dac0000 {
+				compatible = "fsl,imx8ulp-isi";
+				reg = <0x2dac0000 0x10000>;
+				interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&pcc5 IMX8ULP_CLK_ISI>,
+					 <&cgc2 IMX8ULP_CLK_LPAV_AXI_DIV>;
+				clock-names = "axi", "apb";
+				power-domains = <&scmi_devpd IMX8ULP_PD_ISI>;
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						isi_in: endpoint {
+							remote-endpoint = <&mipi_csi_out>;
+						};
+					};
+				};
+			};
+
+			mipi_csi: csi@2daf0000 {
+				compatible = "fsl,imx8ulp-mipi-csi2";
+				reg = <0x2daf0000 0x10000>,
+				      <0x2dad0000 0x10000>;
+				clocks = <&pcc5 IMX8ULP_CLK_CSI>,
+					 <&pcc5 IMX8ULP_CLK_CSI_CLK_ESC>,
+					 <&pcc5 IMX8ULP_CLK_CSI_CLK_UI>,
+					 <&pcc5 IMX8ULP_CLK_CSI_REGS>;
+				clock-names = "core", "esc", "ui", "pclk";
+				assigned-clocks = <&pcc5 IMX8ULP_CLK_CSI>,
+						  <&pcc5 IMX8ULP_CLK_CSI_CLK_ESC>,
+						  <&pcc5 IMX8ULP_CLK_CSI_CLK_UI>,
+						  <&pcc5 IMX8ULP_CLK_CSI_REGS>;
+				assigned-clock-parents = <&cgc2 IMX8ULP_CLK_PLL4_PFD1_DIV1>,
+							 <&cgc2 IMX8ULP_CLK_PLL4_PFD1_DIV2>,
+							 <&cgc2 IMX8ULP_CLK_PLL4_PFD0_DIV1>;
+				assigned-clock-rates = <200000000>,
+						       <80000000>,
+						       <100000000>,
+						       <79200000>;
+				power-domains = <&scmi_devpd IMX8ULP_PD_MIPI_CSI>;
+				resets = <&pcc5 PCC5_CSI_SWRST>,
+					 <&pcc5 PCC5_CSI_REGS_SWRST>;
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mipi_csi_out: endpoint {
+							remote-endpoint = <&isi_in>;
+						};
+					};
+				};
+			};
 		};
 
 		gpiod: gpio@2e200000 {

-- 
2.34.1

Re: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Laurent Pinchart]

Hi Guoniu,

On Thu, Oct 23, 2025 at 05:19:42PM +0800, Guoniu Zhou wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
> 
> The CSI-2 receiver in the i.MX8ULP is almost identical to the version
> present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
> clock as the input clock for its APB interface of Control and Status
> register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
> increase maxItems of Clocks (clock-names) to 4 from 3.  And keep the
> same restriction for existing compatible.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
>  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41 ++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> index 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f24837443f7f4ab6418 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> @@ -20,6 +20,7 @@ properties:
>        - enum:
>            - fsl,imx8mq-mipi-csi2
>            - fsl,imx8qxp-mipi-csi2
> +          - fsl,imx8ulp-mipi-csi2
>        - items:
>            - const: fsl,imx8qm-mipi-csi2
>            - const: fsl,imx8qxp-mipi-csi2
> @@ -39,12 +40,16 @@ properties:
>                       clock that the RX DPHY receives.
>        - description: ui is the pixel clock (phy_ref up to 333Mhz).
>                       See the reference manual for details.
> +      - description: pclk is clock for csr APB interface.
> +    minItems: 3
>  
>    clock-names:
>      items:
>        - const: core
>        - const: esc
>        - const: ui
> +      - const: pclk
> +    minItems: 3
>  
>    power-domains:
>      maxItems: 1
> @@ -130,19 +135,51 @@ allOf:
>          compatible:
>            contains:
>              enum:
> -              - fsl,imx8qxp-mipi-csi2
> +              - fsl,imx8ulp-mipi-csi2
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +        resets:
> +          minItems: 2
> +          maxItems: 2
> +        clocks:
> +          minItems: 4
> +        clock-names:
> +          minItems: 4

Do we need the clock-names constraint ? The DT schemas will enforce that
clocks and clock-names always have the same number of elements.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx8qxp-mipi-csi2
>      then:
>        properties:
>          reg:
>            minItems: 2
>          resets:
>            maxItems: 1
> -    else:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          maxItems: 3
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx8mq-mipi-csi2
> +    then:
>        properties:
>          reg:
>            maxItems: 1
>          resets:
>            minItems: 3
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          maxItems: 3
>        required:
>          - fsl,mipi-phy-gpr
>  

Could you please sort those conditional blocks by alphabetical order of
the compatible strings ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 2/5] media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks

[Laurent Pinchart]

On Thu, Oct 23, 2025 at 05:19:43PM +0800, Guoniu Zhou wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
> 
> Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> 
> No functional changes intended.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
>  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 52 ++++++++-------------------
>  1 file changed, 15 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> index d333ff43539f061b8b9cf88af2cda8c44b3ec2a9..fd202601d401145da8be23df4451f6af660642c5 100644
> --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> @@ -71,21 +71,6 @@ enum {
>  	ST_SUSPENDED	= 4,
>  };
>  
> -enum imx8mq_mipi_csi_clk {
> -	CSI2_CLK_CORE,
> -	CSI2_CLK_ESC,
> -	CSI2_CLK_UI,
> -	CSI2_NUM_CLKS,
> -};
> -
> -static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
> -	[CSI2_CLK_CORE] = "core",
> -	[CSI2_CLK_ESC] = "esc",
> -	[CSI2_CLK_UI] = "ui",
> -};
> -
> -#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> -
>  struct imx8mq_plat_data {
>  	int (*enable)(struct csi_state *state, u32 hs_settle);
>  	void (*disable)(struct csi_state *state);
> @@ -111,7 +96,8 @@ struct csi_state {
>  	struct device *dev;
>  	const struct imx8mq_plat_data *pdata;
>  	void __iomem *regs;
> -	struct clk_bulk_data clks[CSI2_NUM_CLKS];
> +	struct clk_bulk_data *clks;
> +	int num_clks;
>  	struct reset_control *rst;
>  	struct regulator *mipi_phy_regulator;
>  
> @@ -384,24 +370,16 @@ static void imx8mq_mipi_csi_set_params(struct csi_state *state)
>  			      CSI2RX_SEND_LEVEL);
>  }
>  
> -static int imx8mq_mipi_csi_clk_enable(struct csi_state *state)
> -{
> -	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
> -}
> -
> -static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
> +static struct clk *find_esc_clk(struct csi_state *state)

This is one of the reasons why I don't like devm_clk_bulk_get_all(). I
won't object to this patch, but I don't like it. At the very lest, you
should look up the clock at probe time and cache it in the
imx8mq_plat_data structure, to avoid looking it up multiple times at
runtime.

>  {
> -	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
> -}
> -
> -static int imx8mq_mipi_csi_clk_get(struct csi_state *state)
> -{
> -	unsigned int i;
> +	int i;
>  
> -	for (i = 0; i < CSI2_NUM_CLKS; i++)
> -		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
> +	for (i = 0; i < state->num_clks; i++) {

Make state->num_clks unsigned instead of making i signed.

> +		if (!strcmp(state->clks[i].id, "esc"))
> +			return state->clks[i].clk;
> +	}
>  
> -	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
> +	return NULL;

This needs to become a probe error.

>  }
>  
>  static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
> @@ -456,7 +434,7 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
>  	 * documentation recommends picking a value away from the boundaries.
>  	 * Let's pick the average.
>  	 */
> -	esc_clk_rate = clk_get_rate(state->clks[CSI2_CLK_ESC].clk);
> +	esc_clk_rate = clk_get_rate(find_esc_clk(state));
>  	if (!esc_clk_rate) {
>  		dev_err(state->dev, "Could not get esc clock rate.\n");
>  		return -EINVAL;
> @@ -783,7 +761,7 @@ static void imx8mq_mipi_csi_pm_suspend(struct device *dev)
>  
>  	if (state->state & ST_POWERED) {
>  		imx8mq_mipi_csi_stop_stream(state);
> -		imx8mq_mipi_csi_clk_disable(state);
> +		clk_bulk_disable_unprepare(state->num_clks, state->clks);
>  		state->state &= ~ST_POWERED;
>  	}
>  
> @@ -801,7 +779,7 @@ static int imx8mq_mipi_csi_pm_resume(struct device *dev)
>  
>  	if (!(state->state & ST_POWERED)) {
>  		state->state |= ST_POWERED;
> -		ret = imx8mq_mipi_csi_clk_enable(state);
> +		ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
>  	}
>  	if (state->state & ST_STREAMING) {
>  		sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> @@ -1027,9 +1005,9 @@ static int imx8mq_mipi_csi_probe(struct platform_device *pdev)
>  	if (IS_ERR(state->regs))
>  		return PTR_ERR(state->regs);
>  
> -	ret = imx8mq_mipi_csi_clk_get(state);
> -	if (ret < 0)
> -		return ret;
> +	state->num_clks = devm_clk_bulk_get_all(dev, &state->clks);
> +	if (state->num_clks < 0)
> +		return dev_err_probe(dev, state->num_clks, "Failed to get clocks\n");
>  
>  	platform_set_drvdata(pdev, &state->sd);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 3/5] media: imx8mq-mipi-csi2: Explicitly release reset

[Laurent Pinchart]

On Thu, Oct 23, 2025 at 05:19:44PM +0800, Guoniu Zhou wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
> 
> Call reset_control_deassert() to explicitly release reset to make sure
> reset bits are cleared since platform like i.MX8ULP can't clear reset
> bits automatically.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>

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

> ---
>  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> index fd202601d401145da8be23df4451f6af660642c5..fd788a7f48e5feeff658e3d2347db6fefca5d0cf 100644
> --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> @@ -337,18 +337,14 @@ static int imx8mq_mipi_csi_sw_reset(struct csi_state *state)
>  {
>  	int ret;
>  
> -	/*
> -	 * these are most likely self-clearing reset bits. to make it
> -	 * more clear, the reset-imx7 driver should implement the
> -	 * .reset() operation.
> -	 */
>  	ret = reset_control_assert(state->rst);
>  	if (ret < 0) {
>  		dev_err(state->dev, "Failed to assert resets: %d\n", ret);
>  		return ret;
>  	}
>  
> -	return 0;
> +	/* Explicitly release reset to make sure reset bits are cleared. */
> +	return reset_control_deassert(state->rst);
>  }
>  
>  static void imx8mq_mipi_csi_set_params(struct csi_state *state)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 4/5] media: imx8mq-mipi-csi2: Add support for i.MX8ULP

[Laurent Pinchart]

On Thu, Oct 23, 2025 at 05:19:45PM +0800, Guoniu Zhou wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
> 
> The CSI-2 receiver in i.MX8ULP is almost same as i.MX8QXP/QM except
> clocks and resets, so add compatible string for i.MX8ULP to handle
> the difference and reuse platform data of i.MX8QXP/QM.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>

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

> ---
>  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> index fd788a7f48e5feeff658e3d2347db6fefca5d0cf..d8fadb0f1b6b670110ee98a74cffd56a6c96592b 100644
> --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> @@ -1069,6 +1069,7 @@ static void imx8mq_mipi_csi_remove(struct platform_device *pdev)
>  static const struct of_device_id imx8mq_mipi_csi_of_match[] = {
>  	{ .compatible = "fsl,imx8mq-mipi-csi2", .data = &imx8mq_data },
>  	{ .compatible = "fsl,imx8qxp-mipi-csi2", .data = &imx8qxp_data },
> +	{ .compatible = "fsl,imx8ulp-mipi-csi2", .data = &imx8qxp_data },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, imx8mq_mipi_csi_of_match);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 5/5] arm64: dts: imx8ulp: Add CSI and ISI Nodes

[Laurent Pinchart]

On Thu, Oct 23, 2025 at 05:19:46PM +0800, Guoniu Zhou wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
> 
> The CSI-2 in the i.MX8ULP is almost identical to the version present
> in the i.MX8QXP/QM and is routed to the ISI. Add both the ISI and CSI
> nodes and mark them as disabled by default since capture is dependent
> on an attached camera.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>

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

> ---
>  arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 67 ++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index 13b01f3aa2a4950c37e72e04f6bfb5995dc19178..99271d55dfb60ed2cbfe664d928be179eb257674 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> @@ -7,6 +7,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/power/imx8ulp-power.h>
> +#include <dt-bindings/reset/imx8ulp-pcc-reset.h>
>  #include <dt-bindings/thermal/thermal.h>
>  
>  #include "imx8ulp-pinfunc.h"
> @@ -842,6 +843,72 @@ spdif: spdif@2dab0000 {
>  				dma-names = "rx", "tx";
>  				status = "disabled";
>  			};
> +
> +			isi: isi@2dac0000 {
> +				compatible = "fsl,imx8ulp-isi";
> +				reg = <0x2dac0000 0x10000>;
> +				interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&pcc5 IMX8ULP_CLK_ISI>,
> +					 <&cgc2 IMX8ULP_CLK_LPAV_AXI_DIV>;
> +				clock-names = "axi", "apb";
> +				power-domains = <&scmi_devpd IMX8ULP_PD_ISI>;
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						isi_in: endpoint {
> +							remote-endpoint = <&mipi_csi_out>;
> +						};
> +					};
> +				};
> +			};
> +
> +			mipi_csi: csi@2daf0000 {
> +				compatible = "fsl,imx8ulp-mipi-csi2";
> +				reg = <0x2daf0000 0x10000>,
> +				      <0x2dad0000 0x10000>;
> +				clocks = <&pcc5 IMX8ULP_CLK_CSI>,
> +					 <&pcc5 IMX8ULP_CLK_CSI_CLK_ESC>,
> +					 <&pcc5 IMX8ULP_CLK_CSI_CLK_UI>,
> +					 <&pcc5 IMX8ULP_CLK_CSI_REGS>;
> +				clock-names = "core", "esc", "ui", "pclk";
> +				assigned-clocks = <&pcc5 IMX8ULP_CLK_CSI>,
> +						  <&pcc5 IMX8ULP_CLK_CSI_CLK_ESC>,
> +						  <&pcc5 IMX8ULP_CLK_CSI_CLK_UI>,
> +						  <&pcc5 IMX8ULP_CLK_CSI_REGS>;
> +				assigned-clock-parents = <&cgc2 IMX8ULP_CLK_PLL4_PFD1_DIV1>,
> +							 <&cgc2 IMX8ULP_CLK_PLL4_PFD1_DIV2>,
> +							 <&cgc2 IMX8ULP_CLK_PLL4_PFD0_DIV1>;
> +				assigned-clock-rates = <200000000>,
> +						       <80000000>,
> +						       <100000000>,
> +						       <79200000>;
> +				power-domains = <&scmi_devpd IMX8ULP_PD_MIPI_CSI>;
> +				resets = <&pcc5 PCC5_CSI_SWRST>,
> +					 <&pcc5 PCC5_CSI_REGS_SWRST>;
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +
> +						mipi_csi_out: endpoint {
> +							remote-endpoint = <&isi_in>;
> +						};
> +					};
> +				};
> +			};
>  		};
>  
>  		gpiod: gpio@2e200000 {

-- 
Regards,

Laurent Pinchart

RE: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[G.N. Zhou (OSS)]

Hi Laurent,

Thanks for your review.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Monday, October 27, 2025 8:06 AM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Rui Miguel Silva <rmfrfs@gmail.com>; Martin Kepplinger
> <martink@posteo.de>; Purism Kernel Team <kernel@puri.sm>; Mauro
> Carvalho Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Philipp
> Zabel <p.zabel@pengutronix.de>; Frank Li <frank.li@nxp.com>; linux-
> media@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; G.N. Zhou
> <guoniu.zhou@nxp.com>; Conor Dooley <conor.dooley@microchip.com>
> Subject: Re: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add
> i.MX8ULP compatible string
> 
> Hi Guoniu,
> 
> On Thu, Oct 23, 2025 at 05:19:42PM +0800, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > The CSI-2 receiver in the i.MX8ULP is almost identical to the version
> > present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
> > clock as the input clock for its APB interface of Control and Status
> > register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
> > increase maxItems of Clocks (clock-names) to 4 from 3.  And keep the
> > same restriction for existing compatible.
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41
> ++++++++++++++++++++--
> >  1 file changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > index
> >
> 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f2
> 48374
> > 43f7f4ab6418 100644
> > ---
> > a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-
> csi2.yam
> > +++ l
> > @@ -20,6 +20,7 @@ properties:
> >        - enum:
> >            - fsl,imx8mq-mipi-csi2
> >            - fsl,imx8qxp-mipi-csi2
> > +          - fsl,imx8ulp-mipi-csi2
> >        - items:
> >            - const: fsl,imx8qm-mipi-csi2
> >            - const: fsl,imx8qxp-mipi-csi2 @@ -39,12 +40,16 @@
> > properties:
> >                       clock that the RX DPHY receives.
> >        - description: ui is the pixel clock (phy_ref up to 333Mhz).
> >                       See the reference manual for details.
> > +      - description: pclk is clock for csr APB interface.
> > +    minItems: 3
> >
> >    clock-names:
> >      items:
> >        - const: core
> >        - const: esc
> >        - const: ui
> > +      - const: pclk
> > +    minItems: 3
> >
> >    power-domains:
> >      maxItems: 1
> > @@ -130,19 +135,51 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > -              - fsl,imx8qxp-mipi-csi2
> > +              - fsl,imx8ulp-mipi-csi2
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 2
> > +        resets:
> > +          minItems: 2
> > +          maxItems: 2
> > +        clocks:
> > +          minItems: 4
> > +        clock-names:
> > +          minItems: 4
> 
> Do we need the clock-names constraint ? The DT schemas will enforce that
> clocks and clock-names always have the same number of elements.

Yes, I prefer to have it since explicit constraint could make our yaml file more clear to show
our design purpose. In addition, it can also help people who are not familiar with the implicit
constraint in DT schema avoid making mistakes.

> 
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,imx8qxp-mipi-csi2
> >      then:
> >        properties:
> >          reg:
> >            minItems: 2
> >          resets:
> >            maxItems: 1
> > -    else:
> > +        clocks:
> > +          maxItems: 3
> > +        clock-names:
> > +          maxItems: 3
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx8mq-mipi-csi2
> > +    then:
> >        properties:
> >          reg:
> >            maxItems: 1
> >          resets:
> >            minItems: 3
> > +        clocks:
> > +          maxItems: 3
> > +        clock-names:
> > +          maxItems: 3
> >        required:
> >          - fsl,mipi-phy-gpr
> >
> 
> Could you please sort those conditional blocks by alphabetical order of the
> compatible strings ?

Sure

> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH v7 2/5] media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks

[G.N. Zhou (OSS)]

Hi Laurent,

Thanks for your review.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Monday, October 27, 2025 8:12 AM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Rui Miguel Silva <rmfrfs@gmail.com>; Martin Kepplinger
> <martink@posteo.de>; Purism Kernel Team <kernel@puri.sm>; Mauro
> Carvalho Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Philipp
> Zabel <p.zabel@pengutronix.de>; Frank Li <frank.li@nxp.com>; linux-
> media@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; G.N. Zhou
> <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v7 2/5] media: imx8mq-mipi-csi2: Use
> devm_clk_bulk_get_all() to fetch clocks
> 
> On Thu, Oct 23, 2025 at 05:19:43PM +0800, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> >
> > No functional changes intended.
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> >  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 52
> > ++++++++-------------------
> >  1 file changed, 15 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > index
> >
> d333ff43539f061b8b9cf88af2cda8c44b3ec2a9..fd202601d401145da8be23df4
> 451
> > f6af660642c5 100644
> > --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > @@ -71,21 +71,6 @@ enum {
> >  	ST_SUSPENDED	= 4,
> >  };
> >
> > -enum imx8mq_mipi_csi_clk {
> > -	CSI2_CLK_CORE,
> > -	CSI2_CLK_ESC,
> > -	CSI2_CLK_UI,
> > -	CSI2_NUM_CLKS,
> > -};
> > -
> > -static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
> > -	[CSI2_CLK_CORE] = "core",
> > -	[CSI2_CLK_ESC] = "esc",
> > -	[CSI2_CLK_UI] = "ui",
> > -};
> > -
> > -#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> > -
> >  struct imx8mq_plat_data {
> >  	int (*enable)(struct csi_state *state, u32 hs_settle);
> >  	void (*disable)(struct csi_state *state); @@ -111,7 +96,8 @@ struct
> > csi_state {
> >  	struct device *dev;
> >  	const struct imx8mq_plat_data *pdata;
> >  	void __iomem *regs;
> > -	struct clk_bulk_data clks[CSI2_NUM_CLKS];
> > +	struct clk_bulk_data *clks;
> > +	int num_clks;
> >  	struct reset_control *rst;
> >  	struct regulator *mipi_phy_regulator;
> >
> > @@ -384,24 +370,16 @@ static void imx8mq_mipi_csi_set_params(struct
> csi_state *state)
> >  			      CSI2RX_SEND_LEVEL);
> >  }
> >
> > -static int imx8mq_mipi_csi_clk_enable(struct csi_state *state) -{
> > -	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
> > -}
> > -
> > -static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
> > +static struct clk *find_esc_clk(struct csi_state *state)
> 
> This is one of the reasons why I don't like devm_clk_bulk_get_all(). I won't
> object to this patch, but I don't like it. At the very lest, you should look up the
> clock at probe time and cache it in the imx8mq_plat_data structure, to avoid
> looking it up multiple times at runtime.

Ok, but could we cache the clock in the csi_state structure? Because the imx8mq_plat_data
structure instance is const.

> 
> >  {
> > -	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
> > -}
> > -
> > -static int imx8mq_mipi_csi_clk_get(struct csi_state *state) -{
> > -	unsigned int i;
> > +	int i;
> >
> > -	for (i = 0; i < CSI2_NUM_CLKS; i++)
> > -		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
> > +	for (i = 0; i < state->num_clks; i++) {
> 
> Make state->num_clks unsigned instead of making i signed.
> 
> > +		if (!strcmp(state->clks[i].id, "esc"))
> > +			return state->clks[i].clk;
> > +	}
> >
> > -	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
> > +	return NULL;
> 
> This needs to become a probe error.

Got it, will update in next version.

> 
> >  }
> >
> >  static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state, @@
> > -456,7 +434,7 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state
> *state,
> >  	 * documentation recommends picking a value away from the
> boundaries.
> >  	 * Let's pick the average.
> >  	 */
> > -	esc_clk_rate = clk_get_rate(state->clks[CSI2_CLK_ESC].clk);
> > +	esc_clk_rate = clk_get_rate(find_esc_clk(state));
> >  	if (!esc_clk_rate) {
> >  		dev_err(state->dev, "Could not get esc clock rate.\n");
> >  		return -EINVAL;
> > @@ -783,7 +761,7 @@ static void imx8mq_mipi_csi_pm_suspend(struct
> > device *dev)
> >
> >  	if (state->state & ST_POWERED) {
> >  		imx8mq_mipi_csi_stop_stream(state);
> > -		imx8mq_mipi_csi_clk_disable(state);
> > +		clk_bulk_disable_unprepare(state->num_clks, state->clks);
> >  		state->state &= ~ST_POWERED;
> >  	}
> >
> > @@ -801,7 +779,7 @@ static int imx8mq_mipi_csi_pm_resume(struct
> device
> > *dev)
> >
> >  	if (!(state->state & ST_POWERED)) {
> >  		state->state |= ST_POWERED;
> > -		ret = imx8mq_mipi_csi_clk_enable(state);
> > +		ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
> >  	}
> >  	if (state->state & ST_STREAMING) {
> >  		sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> > @@ -1027,9 +1005,9 @@ static int imx8mq_mipi_csi_probe(struct
> platform_device *pdev)
> >  	if (IS_ERR(state->regs))
> >  		return PTR_ERR(state->regs);
> >
> > -	ret = imx8mq_mipi_csi_clk_get(state);
> > -	if (ret < 0)
> > -		return ret;
> > +	state->num_clks = devm_clk_bulk_get_all(dev, &state->clks);
> > +	if (state->num_clks < 0)
> > +		return dev_err_probe(dev, state->num_clks, "Failed to get
> > +clocks\n");
> >
> >  	platform_set_drvdata(pdev, &state->sd);
> >
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH v7 2/5] media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks

[Laurent Pinchart]

On Tue, Nov 04, 2025 at 07:47:43AM +0000, G.N. Zhou (OSS) wrote:
> On Monday, October 27, 2025 8:12 AM, Laurent Pinchart wrote:
> > On Thu, Oct 23, 2025 at 05:19:43PM +0800, Guoniu Zhou wrote:
> > > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > >
> > > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> > >
> > > No functional changes intended.
> > >
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > ---
> > >  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 52
> > > ++++++++-------------------
> > >  1 file changed, 15 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > > b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > > index d333ff43539f061b8b9cf88af2cda8c44b3ec2a9..fd202601d401145da8be23df4451
> > > f6af660642c5 100644
> > > --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > > +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > > @@ -71,21 +71,6 @@ enum {
> > >  	ST_SUSPENDED	= 4,
> > >  };
> > >
> > > -enum imx8mq_mipi_csi_clk {
> > > -	CSI2_CLK_CORE,
> > > -	CSI2_CLK_ESC,
> > > -	CSI2_CLK_UI,
> > > -	CSI2_NUM_CLKS,
> > > -};
> > > -
> > > -static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
> > > -	[CSI2_CLK_CORE] = "core",
> > > -	[CSI2_CLK_ESC] = "esc",
> > > -	[CSI2_CLK_UI] = "ui",
> > > -};
> > > -
> > > -#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> > > -
> > >  struct imx8mq_plat_data {
> > >  	int (*enable)(struct csi_state *state, u32 hs_settle);
> > >  	void (*disable)(struct csi_state *state); @@ -111,7 +96,8 @@ struct csi_state {
> > >  	struct device *dev;
> > >  	const struct imx8mq_plat_data *pdata;
> > >  	void __iomem *regs;
> > > -	struct clk_bulk_data clks[CSI2_NUM_CLKS];
> > > +	struct clk_bulk_data *clks;
> > > +	int num_clks;
> > >  	struct reset_control *rst;
> > >  	struct regulator *mipi_phy_regulator;
> > >
> > > @@ -384,24 +370,16 @@ static void imx8mq_mipi_csi_set_params(struct csi_state *state)
> > >  			      CSI2RX_SEND_LEVEL);
> > >  }
> > >
> > > -static int imx8mq_mipi_csi_clk_enable(struct csi_state *state) -{
> > > -	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
> > > -}
> > > -
> > > -static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
> > > +static struct clk *find_esc_clk(struct csi_state *state)
> > 
> > This is one of the reasons why I don't like devm_clk_bulk_get_all(). I won't
> > object to this patch, but I don't like it. At the very lest, you should look up the
> > clock at probe time and cache it in the imx8mq_plat_data structure, to avoid
> > looking it up multiple times at runtime.
> 
> Ok, but could we cache the clock in the csi_state structure? Because the imx8mq_plat_data
> structure instance is const.

Yes sorry that's what I meant.

> > >  {
> > > -	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
> > > -}
> > > -
> > > -static int imx8mq_mipi_csi_clk_get(struct csi_state *state) -{
> > > -	unsigned int i;
> > > +	int i;
> > >
> > > -	for (i = 0; i < CSI2_NUM_CLKS; i++)
> > > -		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
> > > +	for (i = 0; i < state->num_clks; i++) {
> > 
> > Make state->num_clks unsigned instead of making i signed.
> > 
> > > +		if (!strcmp(state->clks[i].id, "esc"))
> > > +			return state->clks[i].clk;
> > > +	}
> > >
> > > -	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
> > > +	return NULL;
> > 
> > This needs to become a probe error.
> 
> Got it, will update in next version.
> 
> > >  }
> > >
> > >  static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
> > > @@ -456,7 +434,7 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
> > >  	 * documentation recommends picking a value away from the boundaries.
> > >  	 * Let's pick the average.
> > >  	 */
> > > -	esc_clk_rate = clk_get_rate(state->clks[CSI2_CLK_ESC].clk);
> > > +	esc_clk_rate = clk_get_rate(find_esc_clk(state));
> > >  	if (!esc_clk_rate) {
> > >  		dev_err(state->dev, "Could not get esc clock rate.\n");
> > >  		return -EINVAL;
> > > @@ -783,7 +761,7 @@ static void imx8mq_mipi_csi_pm_suspend(struct device *dev)
> > >
> > >  	if (state->state & ST_POWERED) {
> > >  		imx8mq_mipi_csi_stop_stream(state);
> > > -		imx8mq_mipi_csi_clk_disable(state);
> > > +		clk_bulk_disable_unprepare(state->num_clks, state->clks);
> > >  		state->state &= ~ST_POWERED;
> > >  	}
> > >
> > > @@ -801,7 +779,7 @@ static int imx8mq_mipi_csi_pm_resume(struct device *dev)
> > >
> > >  	if (!(state->state & ST_POWERED)) {
> > >  		state->state |= ST_POWERED;
> > > -		ret = imx8mq_mipi_csi_clk_enable(state);
> > > +		ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
> > >  	}
> > >  	if (state->state & ST_STREAMING) {
> > >  		sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> > > @@ -1027,9 +1005,9 @@ static int imx8mq_mipi_csi_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(state->regs))
> > >  		return PTR_ERR(state->regs);
> > >
> > > -	ret = imx8mq_mipi_csi_clk_get(state);
> > > -	if (ret < 0)
> > > -		return ret;
> > > +	state->num_clks = devm_clk_bulk_get_all(dev, &state->clks);
> > > +	if (state->num_clks < 0)
> > > +		return dev_err_probe(dev, state->num_clks, "Failed to get
> > > +clocks\n");
> > >
> > >  	platform_set_drvdata(pdev, &state->sd);
> > >

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Frank Li]

On Mon, Oct 27, 2025 at 02:05:37AM +0200, Laurent Pinchart wrote:
> Hi Guoniu,
>
> On Thu, Oct 23, 2025 at 05:19:42PM +0800, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > The CSI-2 receiver in the i.MX8ULP is almost identical to the version
> > present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
> > clock as the input clock for its APB interface of Control and Status
> > register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
> > increase maxItems of Clocks (clock-names) to 4 from 3.  And keep the
> > same restriction for existing compatible.
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41 ++++++++++++++++++++--
> >  1 file changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > index 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f24837443f7f4ab6418 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > @@ -20,6 +20,7 @@ properties:
> >        - enum:
> >            - fsl,imx8mq-mipi-csi2
> >            - fsl,imx8qxp-mipi-csi2
> > +          - fsl,imx8ulp-mipi-csi2
> >        - items:
> >            - const: fsl,imx8qm-mipi-csi2
> >            - const: fsl,imx8qxp-mipi-csi2
> > @@ -39,12 +40,16 @@ properties:
> >                       clock that the RX DPHY receives.
> >        - description: ui is the pixel clock (phy_ref up to 333Mhz).
> >                       See the reference manual for details.
> > +      - description: pclk is clock for csr APB interface.
> > +    minItems: 3
> >
> >    clock-names:
> >      items:
> >        - const: core
> >        - const: esc
> >        - const: ui
> > +      - const: pclk
> > +    minItems: 3
> >
> >    power-domains:
> >      maxItems: 1
> > @@ -130,19 +135,51 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > -              - fsl,imx8qxp-mipi-csi2
> > +              - fsl,imx8ulp-mipi-csi2
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 2
> > +        resets:
> > +          minItems: 2
> > +          maxItems: 2
> > +        clocks:
> > +          minItems: 4
> > +        clock-names:
> > +          minItems: 4
>
> Do we need the clock-names constraint ? The DT schemas will enforce that
> clocks and clock-names always have the same number of elements.
>

clock-names list already restrict at top section

clock-names:
  items:
    - const: core
    - const: esc
    - const: ui
    - const: pclk
  minItems: 3

Here just restrict need 4 clocks, instead 3 clock for fsl,imx8ulp-mipi-csi2

Frank
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,imx8qxp-mipi-csi2
> >      then:
> >        properties:
> >          reg:
> >            minItems: 2
> >          resets:
> >            maxItems: 1
> > -    else:
> > +        clocks:
> > +          maxItems: 3
> > +        clock-names:
> > +          maxItems: 3
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx8mq-mipi-csi2
> > +    then:
> >        properties:
> >          reg:
> >            maxItems: 1
> >          resets:
> >            minItems: 3
> > +        clocks:
> > +          maxItems: 3
> > +        clock-names:
> > +          maxItems: 3
> >        required:
> >          - fsl,mipi-phy-gpr
> >
>
> Could you please sort those conditional blocks by alphabetical order of
> the compatible strings ?
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Laurent Pinchart]

On Tue, Nov 11, 2025 at 03:47:51PM -0500, Frank Li wrote:
> On Mon, Oct 27, 2025 at 02:05:37AM +0200, Laurent Pinchart wrote:
> > On Thu, Oct 23, 2025 at 05:19:42PM +0800, Guoniu Zhou wrote:
> > > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > >
> > > The CSI-2 receiver in the i.MX8ULP is almost identical to the version
> > > present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
> > > clock as the input clock for its APB interface of Control and Status
> > > register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
> > > increase maxItems of Clocks (clock-names) to 4 from 3.  And keep the
> > > same restriction for existing compatible.
> > >
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > ---
> > >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41 ++++++++++++++++++++--
> > >  1 file changed, 39 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > index 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f24837443f7f4ab6418 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > @@ -20,6 +20,7 @@ properties:
> > >        - enum:
> > >            - fsl,imx8mq-mipi-csi2
> > >            - fsl,imx8qxp-mipi-csi2
> > > +          - fsl,imx8ulp-mipi-csi2
> > >        - items:
> > >            - const: fsl,imx8qm-mipi-csi2
> > >            - const: fsl,imx8qxp-mipi-csi2
> > > @@ -39,12 +40,16 @@ properties:
> > >                       clock that the RX DPHY receives.
> > >        - description: ui is the pixel clock (phy_ref up to 333Mhz).
> > >                       See the reference manual for details.
> > > +      - description: pclk is clock for csr APB interface.
> > > +    minItems: 3
> > >
> > >    clock-names:
> > >      items:
> > >        - const: core
> > >        - const: esc
> > >        - const: ui
> > > +      - const: pclk
> > > +    minItems: 3
> > >
> > >    power-domains:
> > >      maxItems: 1
> > > @@ -130,19 +135,51 @@ allOf:
> > >          compatible:
> > >            contains:
> > >              enum:
> > > -              - fsl,imx8qxp-mipi-csi2
> > > +              - fsl,imx8ulp-mipi-csi2
> > > +    then:
> > > +      properties:
> > > +        reg:
> > > +          minItems: 2
> > > +        resets:
> > > +          minItems: 2
> > > +          maxItems: 2
> > > +        clocks:
> > > +          minItems: 4
> > > +        clock-names:
> > > +          minItems: 4
> >
> > Do we need the clock-names constraint ? The DT schemas will enforce that
> > clocks and clock-names always have the same number of elements.
> 
> clock-names list already restrict at top section
> 
> clock-names:
>   items:
>     - const: core
>     - const: esc
>     - const: ui
>     - const: pclk
>   minItems: 3
> 
> Here just restrict need 4 clocks, instead 3 clock for fsl,imx8ulp-mipi-csi2

I understand that. My point was that the dt-schema will always verify
that the number of clocks items is equal to the number of clock-names
items. That's a constraint enforced by the core schemas. As
clocks: minItems is set to 4, the clock-names: minItems constraint is
redundant.

> > > +
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: fsl,imx8qxp-mipi-csi2
> > >      then:
> > >        properties:
> > >          reg:
> > >            minItems: 2
> > >          resets:
> > >            maxItems: 1
> > > -    else:
> > > +        clocks:
> > > +          maxItems: 3
> > > +        clock-names:
> > > +          maxItems: 3
> > > +
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - fsl,imx8mq-mipi-csi2
> > > +    then:
> > >        properties:
> > >          reg:
> > >            maxItems: 1
> > >          resets:
> > >            minItems: 3
> > > +        clocks:
> > > +          maxItems: 3
> > > +        clock-names:
> > > +          maxItems: 3
> > >        required:
> > >          - fsl,mipi-phy-gpr
> > >
> >
> > Could you please sort those conditional blocks by alphabetical order of
> > the compatible strings ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Frank Li]

On Tue, Nov 11, 2025 at 11:10:25PM +0200, Laurent Pinchart wrote:
> On Tue, Nov 11, 2025 at 03:47:51PM -0500, Frank Li wrote:
> > On Mon, Oct 27, 2025 at 02:05:37AM +0200, Laurent Pinchart wrote:
> > > On Thu, Oct 23, 2025 at 05:19:42PM +0800, Guoniu Zhou wrote:
> > > > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > >
> > > > The CSI-2 receiver in the i.MX8ULP is almost identical to the version
> > > > present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
> > > > clock as the input clock for its APB interface of Control and Status
> > > > register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
> > > > increase maxItems of Clocks (clock-names) to 4 from 3.  And keep the
> > > > same restriction for existing compatible.
> > > >
> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > > ---
> > > >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41 ++++++++++++++++++++--
> > > >  1 file changed, 39 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > index 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f24837443f7f4ab6418 100644
> > > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > @@ -20,6 +20,7 @@ properties:
> > > >        - enum:
> > > >            - fsl,imx8mq-mipi-csi2
> > > >            - fsl,imx8qxp-mipi-csi2
> > > > +          - fsl,imx8ulp-mipi-csi2
> > > >        - items:
> > > >            - const: fsl,imx8qm-mipi-csi2
> > > >            - const: fsl,imx8qxp-mipi-csi2
> > > > @@ -39,12 +40,16 @@ properties:
> > > >                       clock that the RX DPHY receives.
> > > >        - description: ui is the pixel clock (phy_ref up to 333Mhz).
> > > >                       See the reference manual for details.
> > > > +      - description: pclk is clock for csr APB interface.
> > > > +    minItems: 3
> > > >
> > > >    clock-names:
> > > >      items:
> > > >        - const: core
> > > >        - const: esc
> > > >        - const: ui
> > > > +      - const: pclk
> > > > +    minItems: 3
> > > >
> > > >    power-domains:
> > > >      maxItems: 1
> > > > @@ -130,19 +135,51 @@ allOf:
> > > >          compatible:
> > > >            contains:
> > > >              enum:
> > > > -              - fsl,imx8qxp-mipi-csi2
> > > > +              - fsl,imx8ulp-mipi-csi2
> > > > +    then:
> > > > +      properties:
> > > > +        reg:
> > > > +          minItems: 2
> > > > +        resets:
> > > > +          minItems: 2
> > > > +          maxItems: 2
> > > > +        clocks:
> > > > +          minItems: 4
> > > > +        clock-names:
> > > > +          minItems: 4
> > >
> > > Do we need the clock-names constraint ? The DT schemas will enforce that
> > > clocks and clock-names always have the same number of elements.
> >
> > clock-names list already restrict at top section
> >
> > clock-names:
> >   items:
> >     - const: core
> >     - const: esc
> >     - const: ui
> >     - const: pclk
> >   minItems: 3
> >
> > Here just restrict need 4 clocks, instead 3 clock for fsl,imx8ulp-mipi-csi2
>
> I understand that. My point was that the dt-schema will always verify
> that the number of clocks items is equal to the number of clock-names
> items. That's a constraint enforced by the core schemas. As
> clocks: minItems is set to 4, the clock-names: minItems constraint is
> redundant.

I am not sure when have such features. Previous comments from Rob require
clocks and clock-names keep the same at binding yaml.

https://lore.kernel.org/linux-devicetree/20251031000012.GA466250-robh@kernel.org/

Rob have not said that clock-names can be removed.

Do you have any thread, which inidicate we only need limit clocks at
if-else branch?

I also have not found related commit at
https://github.com/devicetree-org/dt-schema.git

Frank

>
> > > > +
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: fsl,imx8qxp-mipi-csi2
> > > >      then:
> > > >        properties:
> > > >          reg:
> > > >            minItems: 2
> > > >          resets:
> > > >            maxItems: 1
> > > > -    else:
> > > > +        clocks:
> > > > +          maxItems: 3
> > > > +        clock-names:
> > > > +          maxItems: 3
> > > > +
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            enum:
> > > > +              - fsl,imx8mq-mipi-csi2
> > > > +    then:
> > > >        properties:
> > > >          reg:
> > > >            maxItems: 1
> > > >          resets:
> > > >            minItems: 3
> > > > +        clocks:
> > > > +          maxItems: 3
> > > > +        clock-names:
> > > > +          maxItems: 3
> > > >        required:
> > > >          - fsl,mipi-phy-gpr
> > > >
> > >
> > > Could you please sort those conditional blocks by alphabetical order of
> > > the compatible strings ?
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Laurent Pinchart]

On Tue, Nov 11, 2025 at 05:06:02PM -0500, Frank Li wrote:
> On Tue, Nov 11, 2025 at 11:10:25PM +0200, Laurent Pinchart wrote:
> > On Tue, Nov 11, 2025 at 03:47:51PM -0500, Frank Li wrote:
> > > On Mon, Oct 27, 2025 at 02:05:37AM +0200, Laurent Pinchart wrote:
> > > > On Thu, Oct 23, 2025 at 05:19:42PM +0800, Guoniu Zhou wrote:
> > > > > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > > >
> > > > > The CSI-2 receiver in the i.MX8ULP is almost identical to the version
> > > > > present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
> > > > > clock as the input clock for its APB interface of Control and Status
> > > > > register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
> > > > > increase maxItems of Clocks (clock-names) to 4 from 3.  And keep the
> > > > > same restriction for existing compatible.
> > > > >
> > > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > > > ---
> > > > >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 41 ++++++++++++++++++++--
> > > > >  1 file changed, 39 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > > index 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f24837443f7f4ab6418 100644
> > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > > @@ -20,6 +20,7 @@ properties:
> > > > >        - enum:
> > > > >            - fsl,imx8mq-mipi-csi2
> > > > >            - fsl,imx8qxp-mipi-csi2
> > > > > +          - fsl,imx8ulp-mipi-csi2
> > > > >        - items:
> > > > >            - const: fsl,imx8qm-mipi-csi2
> > > > >            - const: fsl,imx8qxp-mipi-csi2
> > > > > @@ -39,12 +40,16 @@ properties:
> > > > >                       clock that the RX DPHY receives.
> > > > >        - description: ui is the pixel clock (phy_ref up to 333Mhz).
> > > > >                       See the reference manual for details.
> > > > > +      - description: pclk is clock for csr APB interface.
> > > > > +    minItems: 3
> > > > >
> > > > >    clock-names:
> > > > >      items:
> > > > >        - const: core
> > > > >        - const: esc
> > > > >        - const: ui
> > > > > +      - const: pclk
> > > > > +    minItems: 3
> > > > >
> > > > >    power-domains:
> > > > >      maxItems: 1
> > > > > @@ -130,19 +135,51 @@ allOf:
> > > > >          compatible:
> > > > >            contains:
> > > > >              enum:
> > > > > -              - fsl,imx8qxp-mipi-csi2
> > > > > +              - fsl,imx8ulp-mipi-csi2
> > > > > +    then:
> > > > > +      properties:
> > > > > +        reg:
> > > > > +          minItems: 2
> > > > > +        resets:
> > > > > +          minItems: 2
> > > > > +          maxItems: 2
> > > > > +        clocks:
> > > > > +          minItems: 4
> > > > > +        clock-names:
> > > > > +          minItems: 4
> > > >
> > > > Do we need the clock-names constraint ? The DT schemas will enforce that
> > > > clocks and clock-names always have the same number of elements.
> > >
> > > clock-names list already restrict at top section
> > >
> > > clock-names:
> > >   items:
> > >     - const: core
> > >     - const: esc
> > >     - const: ui
> > >     - const: pclk
> > >   minItems: 3
> > >
> > > Here just restrict need 4 clocks, instead 3 clock for fsl,imx8ulp-mipi-csi2
> >
> > I understand that. My point was that the dt-schema will always verify
> > that the number of clocks items is equal to the number of clock-names
> > items. That's a constraint enforced by the core schemas. As
> > clocks: minItems is set to 4, the clock-names: minItems constraint is
> > redundant.
> 
> I am not sure when have such features. Previous comments from Rob require
> clocks and clock-names keep the same at binding yaml.
> 
> https://lore.kernel.org/linux-devicetree/20251031000012.GA466250-robh@kernel.org/
> 
> Rob have not said that clock-names can be removed.
> 
> Do you have any thread, which inidicate we only need limit clocks at
> if-else branch?
> 
> I also have not found related commit at
> https://github.com/devicetree-org/dt-schema.git

I think you're right. There are dependencies between clock-names and
clocks described in the DT schemas, but I don't see any automatic
dependency regarding the number of items. I seem to have dreamt this :-/
Sorry about the noise.

> > > > > +
> > > > > +  - if:
> > > > > +      properties:
> > > > > +        compatible:
> > > > > +          contains:
> > > > > +            const: fsl,imx8qxp-mipi-csi2
> > > > >      then:
> > > > >        properties:
> > > > >          reg:
> > > > >            minItems: 2
> > > > >          resets:
> > > > >            maxItems: 1
> > > > > -    else:
> > > > > +        clocks:
> > > > > +          maxItems: 3
> > > > > +        clock-names:
> > > > > +          maxItems: 3
> > > > > +
> > > > > +  - if:
> > > > > +      properties:
> > > > > +        compatible:
> > > > > +          contains:
> > > > > +            enum:
> > > > > +              - fsl,imx8mq-mipi-csi2
> > > > > +    then:
> > > > >        properties:
> > > > >          reg:
> > > > >            maxItems: 1
> > > > >          resets:
> > > > >            minItems: 3
> > > > > +        clocks:
> > > > > +          maxItems: 3
> > > > > +        clock-names:
> > > > > +          maxItems: 3
> > > > >        required:
> > > > >          - fsl,mipi-phy-gpr
> > > > >
> > > >
> > > > Could you please sort those conditional blocks by alphabetical order of
> > > > the compatible strings ?

-- 
Regards,

Laurent Pinchart

RE: [PATCH v7 2/5] media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks

[G.N. Zhou (OSS)]

Hi Laurent,

Thank you for your review.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Monday, October 27, 2025 8:12 AM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Rui Miguel Silva <rmfrfs@gmail.com>; Martin Kepplinger
> <martink@posteo.de>; Purism Kernel Team <kernel@puri.sm>; Mauro
> Carvalho Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Philipp
> Zabel <p.zabel@pengutronix.de>; Frank Li <frank.li@nxp.com>; linux-
> media@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; G.N. Zhou
> <guoniu.zhou@nxp.com>
> Subject: Re: [PATCH v7 2/5] media: imx8mq-mipi-csi2: Use
> devm_clk_bulk_get_all() to fetch clocks
> 
> On Thu, Oct 23, 2025 at 05:19:43PM +0800, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> >
> > No functional changes intended.
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> >  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 52
> > ++++++++-------------------
> >  1 file changed, 15 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > index
> >
> d333ff43539f061b8b9cf88af2cda8c44b3ec2a9..fd202601d401145da8be23df4
> 451
> > f6af660642c5 100644
> > --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > @@ -71,21 +71,6 @@ enum {
> >  	ST_SUSPENDED	= 4,
> >  };
> >
> > -enum imx8mq_mipi_csi_clk {
> > -	CSI2_CLK_CORE,
> > -	CSI2_CLK_ESC,
> > -	CSI2_CLK_UI,
> > -	CSI2_NUM_CLKS,
> > -};
> > -
> > -static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
> > -	[CSI2_CLK_CORE] = "core",
> > -	[CSI2_CLK_ESC] = "esc",
> > -	[CSI2_CLK_UI] = "ui",
> > -};
> > -
> > -#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> > -
> >  struct imx8mq_plat_data {
> >  	int (*enable)(struct csi_state *state, u32 hs_settle);
> >  	void (*disable)(struct csi_state *state); @@ -111,7 +96,8 @@ struct
> > csi_state {
> >  	struct device *dev;
> >  	const struct imx8mq_plat_data *pdata;
> >  	void __iomem *regs;
> > -	struct clk_bulk_data clks[CSI2_NUM_CLKS];
> > +	struct clk_bulk_data *clks;
> > +	int num_clks;
> >  	struct reset_control *rst;
> >  	struct regulator *mipi_phy_regulator;
> >
> > @@ -384,24 +370,16 @@ static void imx8mq_mipi_csi_set_params(struct
> csi_state *state)
> >  			      CSI2RX_SEND_LEVEL);
> >  }
> >
> > -static int imx8mq_mipi_csi_clk_enable(struct csi_state *state) -{
> > -	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
> > -}
> > -
> > -static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
> > +static struct clk *find_esc_clk(struct csi_state *state)
> 
> This is one of the reasons why I don't like devm_clk_bulk_get_all(). I won't
> object to this patch, but I don't like it. At the very lest, you should look up the
> clock at probe time and cache it in the imx8mq_plat_data structure, to avoid
> looking it up multiple times at runtime.
> 
> >  {
> > -	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
> > -}
> > -
> > -static int imx8mq_mipi_csi_clk_get(struct csi_state *state) -{
> > -	unsigned int i;
> > +	int i;
> >
> > -	for (i = 0; i < CSI2_NUM_CLKS; i++)
> > -		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
> > +	for (i = 0; i < state->num_clks; i++) {
> 
> Make state->num_clks unsigned instead of making i signed.

I address the comment in v8(https://lore.kernel.org/all/20251113-csi2_imx8ulp-v8-0-2ebe378f7111@nxp.com/),
but Media CI robot detected some issues as bellow.

# Test static:test-smatch
drivers/media/platform/nxp/imx8mq-mipi-csi2.c:1006 imx8mq_mipi_csi_probe() warn: unsigned 'state->num_clks' is never less than zero.

# Test static:test-coccinelle
./platform/nxp/imx8mq-mipi-csi2.c:1006:5-20: WARNING: Unsigned expression compared with zero: state -> num_clks < 0

I checked the parameters type of all functions which will refer to num_clks, their type is int, so I plan to drop the changes in v8 and
will send v9.

> 
> > +		if (!strcmp(state->clks[i].id, "esc"))
> > +			return state->clks[i].clk;
> > +	}
> >
> > -	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
> > +	return NULL;
> 
> This needs to become a probe error.
> 
> >  }
> >
> >  static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state, @@
> > -456,7 +434,7 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state
> *state,
> >  	 * documentation recommends picking a value away from the
> boundaries.
> >  	 * Let's pick the average.
> >  	 */
> > -	esc_clk_rate = clk_get_rate(state->clks[CSI2_CLK_ESC].clk);
> > +	esc_clk_rate = clk_get_rate(find_esc_clk(state));
> >  	if (!esc_clk_rate) {
> >  		dev_err(state->dev, "Could not get esc clock rate.\n");
> >  		return -EINVAL;
> > @@ -783,7 +761,7 @@ static void imx8mq_mipi_csi_pm_suspend(struct
> > device *dev)
> >
> >  	if (state->state & ST_POWERED) {
> >  		imx8mq_mipi_csi_stop_stream(state);
> > -		imx8mq_mipi_csi_clk_disable(state);
> > +		clk_bulk_disable_unprepare(state->num_clks, state->clks);
> >  		state->state &= ~ST_POWERED;
> >  	}
> >
> > @@ -801,7 +779,7 @@ static int imx8mq_mipi_csi_pm_resume(struct
> device
> > *dev)
> >
> >  	if (!(state->state & ST_POWERED)) {
> >  		state->state |= ST_POWERED;
> > -		ret = imx8mq_mipi_csi_clk_enable(state);
> > +		ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
> >  	}
> >  	if (state->state & ST_STREAMING) {
> >  		sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> > @@ -1027,9 +1005,9 @@ static int imx8mq_mipi_csi_probe(struct
> platform_device *pdev)
> >  	if (IS_ERR(state->regs))
> >  		return PTR_ERR(state->regs);
> >
> > -	ret = imx8mq_mipi_csi_clk_get(state);
> > -	if (ret < 0)
> > -		return ret;
> > +	state->num_clks = devm_clk_bulk_get_all(dev, &state->clks);
> > +	if (state->num_clks < 0)
> > +		return dev_err_probe(dev, state->num_clks, "Failed to get
> > +clocks\n");
> >
> >  	platform_set_drvdata(pdev, &state->sd);
> >
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH v7 2/5] media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks

[Laurent Pinchart]

On Fri, Nov 21, 2025 at 02:21:23AM +0000, G.N. Zhou (OSS) wrote:
> On Monday, October 27, 2025 8:12 AM, Laurent Pinchart wrote:
> > On Thu, Oct 23, 2025 at 05:19:43PM +0800, Guoniu Zhou wrote:
> > > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> > >
> > > Use devm_clk_bulk_get_all() helper to simplify clock handle code.
> > >
> > > No functional changes intended.
> > >
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > ---
> > >  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 52 ++++++++-------------------
> > >  1 file changed, 15 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > > index d333ff43539f061b8b9cf88af2cda8c44b3ec2a9..fd202601d401145da8be23df4451f6af660642c5 100644
> > > --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > > +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > > @@ -71,21 +71,6 @@ enum {
> > >  	ST_SUSPENDED	= 4,
> > >  };
> > >
> > > -enum imx8mq_mipi_csi_clk {
> > > -	CSI2_CLK_CORE,
> > > -	CSI2_CLK_ESC,
> > > -	CSI2_CLK_UI,
> > > -	CSI2_NUM_CLKS,
> > > -};
> > > -
> > > -static const char * const imx8mq_mipi_csi_clk_id[CSI2_NUM_CLKS] = {
> > > -	[CSI2_CLK_CORE] = "core",
> > > -	[CSI2_CLK_ESC] = "esc",
> > > -	[CSI2_CLK_UI] = "ui",
> > > -};
> > > -
> > > -#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> > > -
> > >  struct imx8mq_plat_data {
> > >  	int (*enable)(struct csi_state *state, u32 hs_settle);
> > >  	void (*disable)(struct csi_state *state);
> > > @@ -111,7 +96,8 @@ struct csi_state {
> > >  	struct device *dev;
> > >  	const struct imx8mq_plat_data *pdata;
> > >  	void __iomem *regs;
> > > -	struct clk_bulk_data clks[CSI2_NUM_CLKS];
> > > +	struct clk_bulk_data *clks;
> > > +	int num_clks;
> > >  	struct reset_control *rst;
> > >  	struct regulator *mipi_phy_regulator;
> > >
> > > @@ -384,24 +370,16 @@ static void imx8mq_mipi_csi_set_params(struct csi_state *state)
> > >  			      CSI2RX_SEND_LEVEL);
> > >  }
> > >
> > > -static int imx8mq_mipi_csi_clk_enable(struct csi_state *state) -{
> > > -	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
> > > -}
> > > -
> > > -static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
> > > +static struct clk *find_esc_clk(struct csi_state *state)
> > 
> > This is one of the reasons why I don't like devm_clk_bulk_get_all(). I won't
> > object to this patch, but I don't like it. At the very lest, you should look up the
> > clock at probe time and cache it in the imx8mq_plat_data structure, to avoid
> > looking it up multiple times at runtime.
> > 
> > >  {
> > > -	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
> > > -}
> > > -
> > > -static int imx8mq_mipi_csi_clk_get(struct csi_state *state) -{
> > > -	unsigned int i;
> > > +	int i;
> > >
> > > -	for (i = 0; i < CSI2_NUM_CLKS; i++)
> > > -		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
> > > +	for (i = 0; i < state->num_clks; i++) {
> > 
> > Make state->num_clks unsigned instead of making i signed.
> 
> I address the comment in v8(https://lore.kernel.org/all/20251113-csi2_imx8ulp-v8-0-2ebe378f7111@nxp.com/),
> but Media CI robot detected some issues as bellow.
> 
> # Test static:test-smatch
> drivers/media/platform/nxp/imx8mq-mipi-csi2.c:1006 imx8mq_mipi_csi_probe() warn: unsigned 'state->num_clks' is never less than zero.
> 
> # Test static:test-coccinelle
> ./platform/nxp/imx8mq-mipi-csi2.c:1006:5-20: WARNING: Unsigned expression compared with zero: state -> num_clks < 0
> 
> I checked the parameters type of all functions which will refer to
> num_clks, their type is int, so I plan to drop the changes in v8 and
> will send v9.

You can address the issue in the probe function as follows:

	ret = devm_clk_bulk_get_all(dev, &state->clks);
	if (ret < 0)
		return dev_err_probe(dev, ret, "Failed to get clocks\n");

	state->num_clks = ret;

> > > +		if (!strcmp(state->clks[i].id, "esc"))
> > > +			return state->clks[i].clk;
> > > +	}
> > >
> > > -	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
> > > +	return NULL;
> > 
> > This needs to become a probe error.
> > 
> > >  }
> > >
> > >  static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
> > > @@ -456,7 +434,7 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
> > >  	 * documentation recommends picking a value away from the boundaries.
> > >  	 * Let's pick the average.
> > >  	 */
> > > -	esc_clk_rate = clk_get_rate(state->clks[CSI2_CLK_ESC].clk);
> > > +	esc_clk_rate = clk_get_rate(find_esc_clk(state));
> > >  	if (!esc_clk_rate) {
> > >  		dev_err(state->dev, "Could not get esc clock rate.\n");
> > >  		return -EINVAL;
> > > @@ -783,7 +761,7 @@ static void imx8mq_mipi_csi_pm_suspend(struct device *dev)
> > >
> > >  	if (state->state & ST_POWERED) {
> > >  		imx8mq_mipi_csi_stop_stream(state);
> > > -		imx8mq_mipi_csi_clk_disable(state);
> > > +		clk_bulk_disable_unprepare(state->num_clks, state->clks);
> > >  		state->state &= ~ST_POWERED;
> > >  	}
> > >
> > > @@ -801,7 +779,7 @@ static int imx8mq_mipi_csi_pm_resume(struct device
> > > *dev)
> > >
> > >  	if (!(state->state & ST_POWERED)) {
> > >  		state->state |= ST_POWERED;
> > > -		ret = imx8mq_mipi_csi_clk_enable(state);
> > > +		ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
> > >  	}
> > >  	if (state->state & ST_STREAMING) {
> > >  		sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> > > @@ -1027,9 +1005,9 @@ static int imx8mq_mipi_csi_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(state->regs))
> > >  		return PTR_ERR(state->regs);
> > >
> > > -	ret = imx8mq_mipi_csi_clk_get(state);
> > > -	if (ret < 0)
> > > -		return ret;
> > > +	state->num_clks = devm_clk_bulk_get_all(dev, &state->clks);
> > > +	if (state->num_clks < 0)
> > > +		return dev_err_probe(dev, state->num_clks, "Failed to get clocks\n");
> > >
> > >  	platform_set_drvdata(pdev, &state->sd);
> > >

-- 
Regards,

Laurent Pinchart