[PATCH v5 0/4] Add MIPI CSI-2 support for i.MX8ULP

[PATCH v5 0/4] 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 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 (4):
      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
      arm64: dts: imx8ulp: Add CSI and ISI Nodes

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

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

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

[Guoniu Zhou]

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 existed compatible.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
 .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 46 ++++++++++++++++++++--
 1 file changed, 43 insertions(+), 3 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..412cedddb0efee1a49d1b90b02baa7a625c797ec 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
@@ -21,7 +21,9 @@ properties:
           - fsl,imx8mq-mipi-csi2
           - fsl,imx8qxp-mipi-csi2
       - items:
-          - const: fsl,imx8qm-mipi-csi2
+          - enum:
+              - fsl,imx8qm-mipi-csi2
+              - fsl,imx8ulp-mipi-csi2
           - const: fsl,imx8qxp-mipi-csi2
 
   reg:
@@ -39,12 +41,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 +136,53 @@ 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:
+            enum:
+              - fsl,imx8qm-mipi-csi2
+            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 v5 2/4] media: imx8mq-mipi-csi2: Use devm_clk_bulk_get_all() to fetch clocks

[Guoniu Zhou]

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 3a4645f59a44028fdca82a4d8393e1a0a6ba88f0..2bf11984690af2e687a3217e465697333d9d995d 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 v5 3/4] media: imx8mq-mipi-csi2: Explicitly release reset

[Guoniu Zhou]

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 2bf11984690af2e687a3217e465697333d9d995d..6b83aa85af42e1dac25cf29056863680c1f89402 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 v5 4/4] arm64: dts: imx8ulp: Add CSI and ISI Nodes

[Guoniu Zhou]

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..7981f7dc62f5dfb2dff051e2d91bde6a2498ac13 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", "fsl,imx8qxp-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 v5 3/4] media: imx8mq-mipi-csi2: Explicitly release reset

[Laurent Pinchart]

Hi Guoniu,

Thank you for the patch.

On Mon, Sep 01, 2025 at 02:25:31PM +0800, Guoniu Zhou wrote:
> 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 2bf11984690af2e687a3217e465697333d9d995d..6b83aa85af42e1dac25cf29056863680c1f89402 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.

What happened to this plan, would it be feasible to implement the
.reset() operation in the relevant drivers to be able to use
reset_control_reset() here ?

> -	 */
>  	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 v5 1/4] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Laurent Pinchart]

Hi Guoniu,

Thank you for the patch.

On Mon, Sep 01, 2025 at 02:25:29PM +0800, Guoniu Zhou wrote:
> 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 existed compatible.

s/existed/existing/

> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
>  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 46 ++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 3 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..412cedddb0efee1a49d1b90b02baa7a625c797ec 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> @@ -21,7 +21,9 @@ properties:
>            - fsl,imx8mq-mipi-csi2
>            - fsl,imx8qxp-mipi-csi2
>        - items:
> -          - const: fsl,imx8qm-mipi-csi2
> +          - enum:
> +              - fsl,imx8qm-mipi-csi2
> +              - fsl,imx8ulp-mipi-csi2
>            - const: fsl,imx8qxp-mipi-csi2

According to this, the ULP version is compatible with the QXP version.

>  
>    reg:
> @@ -39,12 +41,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 +136,53 @@ 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

But according to this, the ULP version requires more clocks than the QXP
version.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx8qm-mipi-csi2

QM is compatible with the QXP, so you don't need to list it here.

          contains:
            const: fsl,imx8qxp-mipi-csi2

is enough to cover both.

> +            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
>  

-- 
Regards,

Laurent Pinchart

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

[Frank Li]

On Mon, Sep 01, 2025 at 05:46:10PM +0200, Laurent Pinchart wrote:
> Hi Guoniu,
>
> Thank you for the patch.
>
> On Mon, Sep 01, 2025 at 02:25:29PM +0800, Guoniu Zhou wrote:
> > 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 existed compatible.
>
> s/existed/existing/
>
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 46 ++++++++++++++++++++--
> >  1 file changed, 43 insertions(+), 3 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..412cedddb0efee1a49d1b90b02baa7a625c797ec 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > @@ -21,7 +21,9 @@ properties:
> >            - fsl,imx8mq-mipi-csi2
> >            - fsl,imx8qxp-mipi-csi2
> >        - items:
> > -          - const: fsl,imx8qm-mipi-csi2
> > +          - enum:
> > +              - fsl,imx8qm-mipi-csi2
> > +              - fsl,imx8ulp-mipi-csi2
> >            - const: fsl,imx8qxp-mipi-csi2
>
> According to this, the ULP version is compatible with the QXP version.
>
> >
> >    reg:
> > @@ -39,12 +41,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 +136,53 @@ 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
>
> But according to this, the ULP version requires more clocks than the QXP
> version.

If only clock number difference, generally, it is still compatible and can
be fallback, especialy driver use devm_bulk_clk_get_all().

If driver have not sperated drvdata for it, we can fallback to it. It is
quite common.

Frank

>
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx8qm-mipi-csi2
>
> QM is compatible with the QXP, so you don't need to list it here.
>
>           contains:
>             const: fsl,imx8qxp-mipi-csi2
>
> is enough to cover both.
>
> > +            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
> >
>
> --
> Regards,
>
> Laurent Pinchart

RE: [EXT] Re: [PATCH v5 3/4] media: imx8mq-mipi-csi2: Explicitly release reset

[G.N. Zhou]

Hi Laurent,

Thanks for your review.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Monday, September 1, 2025 11:37 PM
> To: G.N. Zhou <guoniu.zhou@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
> Subject: [EXT] Re: [PATCH v5 3/4] media: imx8mq-mipi-csi2: Explicitly release
> reset
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> Hi Guoniu,
> 
> Thank you for the patch.
> 
> On Mon, Sep 01, 2025 at 02:25:31PM +0800, Guoniu Zhou wrote:
> > 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
> >
> 2bf11984690af2e687a3217e465697333d9d995d..6b83aa85af42e1dac25cf29
> 05686
> > 3680c1f89402 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.
> 
> What happened to this plan, would it be feasible to implement the

Since reset in ULP isn't self-clearing, so need to release the reset before return.
And I think it's no side effect to call reset_control_deassert() here since it makes
more clear and readable about software reset implementation.

> .reset() operation in the relevant drivers to be able to use
> reset_control_reset() here ?

Implement the .reset() operation in in the relevant drivers should have same effect
like here. If you agree, I prefer to use the patch here since less changes usually mean
low risk.

> 
> > -      */
> >       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 v5 1/4] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Laurent Pinchart]

On Mon, Sep 01, 2025 at 09:45:39PM -0400, Frank Li wrote:
> On Mon, Sep 01, 2025 at 05:46:10PM +0200, Laurent Pinchart wrote:
> > On Mon, Sep 01, 2025 at 02:25:29PM +0800, Guoniu Zhou wrote:
> > > 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 existed compatible.
> >
> > s/existed/existing/
> >
> > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > ---
> > >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 46 ++++++++++++++++++++--
> > >  1 file changed, 43 insertions(+), 3 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..412cedddb0efee1a49d1b90b02baa7a625c797ec 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > @@ -21,7 +21,9 @@ properties:
> > >            - fsl,imx8mq-mipi-csi2
> > >            - fsl,imx8qxp-mipi-csi2
> > >        - items:
> > > -          - const: fsl,imx8qm-mipi-csi2
> > > +          - enum:
> > > +              - fsl,imx8qm-mipi-csi2
> > > +              - fsl,imx8ulp-mipi-csi2
> > >            - const: fsl,imx8qxp-mipi-csi2
> >
> > According to this, the ULP version is compatible with the QXP version.
> >
> > >
> > >    reg:
> > > @@ -39,12 +41,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 +136,53 @@ 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
> >
> > But according to this, the ULP version requires more clocks than the QXP
> > version.
> 
> If only clock number difference, generally, it is still compatible and can
> be fallback, especialy driver use devm_bulk_clk_get_all().

That's a driver-specific implementation decision, so I don't think it
should be taken into account to decide on compatibility.

> If driver have not sperated drvdata for it, we can fallback to it. It is
> quite common.
> 
> > > +
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - fsl,imx8qm-mipi-csi2
> >
> > QM is compatible with the QXP, so you don't need to list it here.
> >
> >           contains:
> >             const: fsl,imx8qxp-mipi-csi2
> >
> > is enough to cover both.
> >
> > > +            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
> > >

-- 
Regards,

Laurent Pinchart

Re: [EXT] Re: [PATCH v5 3/4] media: imx8mq-mipi-csi2: Explicitly release reset

[Laurent Pinchart]

On Tue, Sep 02, 2025 at 02:21:58AM +0000, G.N. Zhou wrote:
> On Monday, September 1, 2025 11:37 PM, Laurent Pinchart wrote:
> > On Mon, Sep 01, 2025 at 02:25:31PM +0800, Guoniu Zhou wrote:
> > > 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 2bf11984690af2e687a3217e465697333d9d995d..6b83aa85af42e1dac25cf29056863680c1f89402
> > > 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.
> > 
> > What happened to this plan, would it be feasible to implement the
> 
> Since reset in ULP isn't self-clearing, so need to release the reset before return.
> And I think it's no side effect to call reset_control_deassert() here since it makes
> more clear and readable about software reset implementation.
> 
> > .reset() operation in the relevant drivers to be able to use
> > reset_control_reset() here ?
> 
> Implement the .reset() operation in in the relevant drivers should have same effect
> like here. If you agree, I prefer to use the patch here since less changes usually mean
> low risk.

I'm OK with that.

> > > -      */
> > >       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 v5 1/4] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

[Frank Li]

On Tue, Sep 02, 2025 at 10:35:54AM +0200, Laurent Pinchart wrote:
> On Mon, Sep 01, 2025 at 09:45:39PM -0400, Frank Li wrote:
> > On Mon, Sep 01, 2025 at 05:46:10PM +0200, Laurent Pinchart wrote:
> > > On Mon, Sep 01, 2025 at 02:25:29PM +0800, Guoniu Zhou wrote:
> > > > 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 existed compatible.
> > >
> > > s/existed/existing/
> > >
> > > > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > > > ---
> > > >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml       | 46 ++++++++++++++++++++--
> > > >  1 file changed, 43 insertions(+), 3 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..412cedddb0efee1a49d1b90b02baa7a625c797ec 100644
> > > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > > > @@ -21,7 +21,9 @@ properties:
> > > >            - fsl,imx8mq-mipi-csi2
> > > >            - fsl,imx8qxp-mipi-csi2
> > > >        - items:
> > > > -          - const: fsl,imx8qm-mipi-csi2
> > > > +          - enum:
> > > > +              - fsl,imx8qm-mipi-csi2
> > > > +              - fsl,imx8ulp-mipi-csi2
> > > >            - const: fsl,imx8qxp-mipi-csi2
> > >
> > > According to this, the ULP version is compatible with the QXP version.
> > >
> > > >
> > > >    reg:
> > > > @@ -39,12 +41,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 +136,53 @@ 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
> > >
> > > But according to this, the ULP version requires more clocks than the QXP
> > > version.
> >
> > If only clock number difference, generally, it is still compatible and can
> > be fallback, especialy driver use devm_bulk_clk_get_all().
>
> That's a driver-specific implementation decision, so I don't think it
> should be taken into account to decide on compatibility.

It is easy to follow to decide if fallback to existing compatible string.
If driver can work with fallback string for new compatible string, we can
add it as fallback string.

Use fallback string don't affect ABI if we find new feature or bugs need
handle specific in drivers.

Anyways, at other binding review, most only clk number difference can treat
as back compatible string.

Frank
>
> > If driver have not sperated drvdata for it, we can fallback to it. It is
> > quite common.
> >
> > > > +
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            enum:
> > > > +              - fsl,imx8qm-mipi-csi2
> > >
> > > QM is compatible with the QXP, so you don't need to list it here.
> > >
> > >           contains:
> > >             const: fsl,imx8qxp-mipi-csi2
> > >
> > > is enough to cover both.
> > >
> > > > +            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
> > > >
>
> --
> Regards,
>
> Laurent Pinchart

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

[Krzysztof Kozlowski]

On 02/09/2025 10:35, Laurent Pinchart wrote:
>>>>          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
>>>
>>> But according to this, the ULP version requires more clocks than the QXP
>>> version.
>>
>> If only clock number difference, generally, it is still compatible and can
>> be fallback, especialy driver use devm_bulk_clk_get_all().
> 
> That's a driver-specific implementation decision, so I don't think it
> should be taken into account to decide on compatibility.

The clock inputs do not restrict compatibility. If Linux can use
fallback to bind and operate properly, then it's a strong indication
devices are compatible.

Imagine exactly the same registers, so same programming interface, but
one device takes one more clock which just needs to be enabled through
its lifetime. Such devices are fully compatible, even though clock
inputs differ.

I also wanted to express exactly that case on my slides from OSSE -
slide 28:
https://osseu2025.sched.com/event/25Vsl/dts-101-from-roots-to-trees-aka-devicetree-for-beginners-krzysztof-kozlowski-linaro

(although I focused on reversed case when devices are not compatible,
because that is decisive case).

Best regards,
Krzysztof

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

[Laurent Pinchart]

On Tue, Sep 02, 2025 at 02:26:53PM +0200, Krzysztof Kozlowski wrote:
> On 02/09/2025 10:35, Laurent Pinchart wrote:
> >>>>          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
> >>>
> >>> But according to this, the ULP version requires more clocks than the QXP
> >>> version.
> >>
> >> If only clock number difference, generally, it is still compatible and can
> >> be fallback, especialy driver use devm_bulk_clk_get_all().
> > 
> > That's a driver-specific implementation decision, so I don't think it
> > should be taken into account to decide on compatibility.
> 
> The clock inputs do not restrict compatibility. If Linux can use
> fallback to bind and operate properly, then it's a strong indication
> devices are compatible.
> 
> Imagine exactly the same registers, so same programming interface, but
> one device takes one more clock which just needs to be enabled through
> its lifetime. Such devices are fully compatible, even though clock
> inputs differ.

That's only the case if someone enables the clock, isn't it ? From a DT
binding point of view, how can we know that the extra clock will be
enabled by a component separate from the driver (in this case by the
fact that the devm_bulk_clk_get_all() function gets all clocks) ?

> I also wanted to express exactly that case on my slides from OSSE -
> slide 28:
> https://osseu2025.sched.com/event/25Vsl/dts-101-from-roots-to-trees-aka-devicetree-for-beginners-krzysztof-kozlowski-linaro

Quoting that slide, you wrote

"Two devices are compatible when the new device works with Linux drivers
bound via fallback (old) compatible".

That is clearly the case here for the existing *Linux* driver. But what
if the driver called devm_bulkd_clk_get() with a device-specific list of
clocks ? Or what if the same DT bindings are used on an OS that has no
clk_get_all() equivalent ? This is my concern with declaring those two
devices as compatible: they may be from the point of view of the current
implementation of the corresponding Linux kernel driver, but DT bindings
are not Linux-specific.

Or do DT bindings assume that drivers have to always enable all clocks
declared in DT, even if they don't know what those clocks are ? That
seems error-prone, in quite a few cases drivers need to handle separate
clocks in a device-specific way, with for instance a particular
ordering, preventing them from using devm_bulk_clk_get_all(). If all
drivers are required to manage all clocks declared in DT, this would get
messy quite quickly.

> (although I focused on reversed case when devices are not compatible,
> because that is decisive case).

-- 
Regards,

Laurent Pinchart

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

[Krzysztof Kozlowski]

On 02/09/2025 14:35, Laurent Pinchart wrote:
> On Tue, Sep 02, 2025 at 02:26:53PM +0200, Krzysztof Kozlowski wrote:
>> On 02/09/2025 10:35, Laurent Pinchart wrote:
>>>>>>          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
>>>>>
>>>>> But according to this, the ULP version requires more clocks than the QXP
>>>>> version.
>>>>
>>>> If only clock number difference, generally, it is still compatible and can
>>>> be fallback, especialy driver use devm_bulk_clk_get_all().
>>>
>>> That's a driver-specific implementation decision, so I don't think it
>>> should be taken into account to decide on compatibility.
>>
>> The clock inputs do not restrict compatibility. If Linux can use
>> fallback to bind and operate properly, then it's a strong indication
>> devices are compatible.
>>
>> Imagine exactly the same registers, so same programming interface, but
>> one device takes one more clock which just needs to be enabled through
>> its lifetime. Such devices are fully compatible, even though clock
>> inputs differ.
> 
> That's only the case if someone enables the clock, isn't it ? From a DT
> binding point of view, how can we know that the extra clock will be

We talk about software using the binding in this particular case. Can
the software use fallback? Yes, it can.

> enabled by a component separate from the driver (in this case by the
> fact that the devm_bulk_clk_get_all() function gets all clocks) ?

If you go that way, only 100% identical devices are compatible.

> 
>> I also wanted to express exactly that case on my slides from OSSE -
>> slide 28:
>> https://osseu2025.sched.com/event/25Vsl/dts-101-from-roots-to-trees-aka-devicetree-for-beginners-krzysztof-kozlowski-linaro
> 
> Quoting that slide, you wrote
> 
> "Two devices are compatible when the new device works with Linux drivers
> bound via fallback (old) compatible".
> 
> That is clearly the case here for the existing *Linux* driver. But what
> if the driver called devm_bulkd_clk_get() with a device-specific list of
> clocks ? Or what if the same DT bindings are used on an OS that has no
> clk_get_all() equivalent ? This is my concern with declaring those two
> devices as compatible: they may be from the point of view of the current
> implementation of the corresponding Linux kernel driver, but DT bindings
> are not Linux-specific.

It seems you think of compatibility as new device is compatible with old
kernel, e.g. one not requesting that clock. We don't talk about such case.

> 
> Or do DT bindings assume that drivers have to always enable all clocks
> declared in DT, even if they don't know what those clocks are ? That
> seems error-prone, in quite a few cases drivers need to handle separate
> clocks in a device-specific way, with for instance a particular
> ordering, preventing them from using devm_bulk_clk_get_all(). If all
> drivers are required to manage all clocks declared in DT, this would get
> messy quite quickly.
> 
I don't really want to dive into such specifics, because it is
impossible to create a generic rule of out. We decide here about
programming interface mostly. Can Linux use the one from fallback-device
to properly operate the new one? Can the same driver bind to fallback
and operate the new device?

If you enable clock by clock for whatever reason, e.g. very specific
programming power up sequence, then answer would be: no, Linux cannot
use fallback because handling clocks differ.

Best regards,
Krzysztof