[PATCH v2 00/10] Add support for i.MX94 DCIF

[PATCH v2 00/10] Add support for i.MX94 DCIF

[Laurentiu Palcu]

Hi,

This patch-set adds support for the i.MX94 Display Control Interface.
It applies on top of Peng Fan's BLK_CTL patches: [1].

Also, included in the patch-set are a few extra patches that the DCIF
driver depends on for functioning properly:
 * 1/10 - 2/10 : BLK_CTL driver fixes;
 * 3/10 - 5/10 : add support for i.MX94 to fsl-ldb driver. It also
			     contains a patch (4/8) from Liu Ying that was already reviewed
			     and was part of another patch-set ([2]), but was never merged;

v2:
 * reworked the BLK_CTL patch and split in 2 to make it easier for
   review;
 * split the dts and dtsi patch in 2 separate ones;
 * addressed Frank's comments in DCIF driver;
 * addressed Rob's comments for the bindings files;
 * addressed a couple of checkpatch issues;

Thanks,
Laurentiu

[1] https://lkml.org/lkml/2025/7/7/78
[2] https://lkml.org/lkml/2024/11/14/262

Laurentiu Palcu (8):
  clk: imx95-blk-ctl: Save platform data in imx95_blk_ctl structure
  clk: imx95-blk-ctl: Save/restore registers when RPM routines are
    called
  dt-bindings: display: fsl,ldb: Add i.MX94 LDB
  drm/bridge: fsl-ldb: Add support for i.MX94
  dt-bindings: display: imx: Add bindings for i.MX94 DCIF
  arm64: dts: imx943: Add display pipeline nodes
  arm64: dts: imx943-evk: Add display support using IT6263
  MAINTAINERS: Add entry for i.MX94 DCIF driver

Liu Ying (1):
  drm/bridge: fsl-ldb: Get the next non-panel bridge

Sandor Yu (1):
  drm/imx: Add support for i.MX94 DCIF

 .../bindings/display/bridge/fsl,ldb.yaml      |   2 +
 .../bindings/display/imx/nxp,imx94-dcif.yaml  |  93 +++
 MAINTAINERS                                   |   9 +
 arch/arm64/boot/dts/freescale/imx943-evk.dts  | 123 ++++
 arch/arm64/boot/dts/freescale/imx943.dtsi     |  56 +-
 drivers/clk/imx/clk-imx95-blk-ctl.c           |  51 +-
 drivers/gpu/drm/bridge/fsl-ldb.c              |  47 +-
 drivers/gpu/drm/imx/Kconfig                   |   1 +
 drivers/gpu/drm/imx/Makefile                  |   1 +
 drivers/gpu/drm/imx/dcif/Kconfig              |  15 +
 drivers/gpu/drm/imx/dcif/Makefile             |   5 +
 drivers/gpu/drm/imx/dcif/dcif-crc.c           | 211 ++++++
 drivers/gpu/drm/imx/dcif/dcif-crc.h           |  52 ++
 drivers/gpu/drm/imx/dcif/dcif-crtc.c          | 695 ++++++++++++++++++
 drivers/gpu/drm/imx/dcif/dcif-drv.c           | 282 +++++++
 drivers/gpu/drm/imx/dcif/dcif-drv.h           |  87 +++
 drivers/gpu/drm/imx/dcif/dcif-kms.c           | 101 +++
 drivers/gpu/drm/imx/dcif/dcif-plane.c         | 269 +++++++
 drivers/gpu/drm/imx/dcif/dcif-reg.h           | 266 +++++++
 19 files changed, 2314 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/imx/nxp,imx94-dcif.yaml
 create mode 100644 drivers/gpu/drm/imx/dcif/Kconfig
 create mode 100644 drivers/gpu/drm/imx/dcif/Makefile
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-crc.c
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-crc.h
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-crtc.c
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-drv.c
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-drv.h
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-kms.c
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-plane.c
 create mode 100644 drivers/gpu/drm/imx/dcif/dcif-reg.h

-- 
2.34.1

[PATCH v2 01/10] clk: imx95-blk-ctl: Save platform data in imx95_blk_ctl structure

[Laurentiu Palcu]

Currently, besides probe(), the platform data is read in both suspend()
and resume(). Let's avoid this by making pdata a member of imx95_blk_ctl
structure.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/clk/imx/clk-imx95-blk-ctl.c | 36 +++++++++++------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
index 7e88877a62451..c72debaf3a60b 100644
--- a/drivers/clk/imx/clk-imx95-blk-ctl.c
+++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
@@ -36,6 +36,7 @@ struct imx95_blk_ctl {
 	void __iomem *base;
 	/* clock gate register */
 	u32 clk_reg_restore;
+	const struct imx95_blk_ctl_dev_data *pdata;
 };
 
 struct imx95_blk_ctl_clk_dev_data {
@@ -349,7 +350,6 @@ static const struct imx95_blk_ctl_dev_data imx94_dispmix_csr_dev_data = {
 static int imx95_bc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	const struct imx95_blk_ctl_dev_data *bc_data;
 	struct imx95_blk_ctl *bc;
 	struct clk_hw_onecell_data *clk_hw_data;
 	struct clk_hw **hws;
@@ -379,25 +379,25 @@ static int imx95_bc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	bc_data = of_device_get_match_data(dev);
-	if (!bc_data)
+	bc->pdata = of_device_get_match_data(dev);
+	if (!bc->pdata)
 		return devm_of_platform_populate(dev);
 
-	clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, bc_data->num_clks),
+	clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, bc->pdata->num_clks),
 				   GFP_KERNEL);
 	if (!clk_hw_data)
 		return -ENOMEM;
 
-	if (bc_data->rpm_enabled) {
+	if (bc->pdata->rpm_enabled) {
 		devm_pm_runtime_enable(&pdev->dev);
 		pm_runtime_resume_and_get(&pdev->dev);
 	}
 
-	clk_hw_data->num = bc_data->num_clks;
+	clk_hw_data->num = bc->pdata->num_clks;
 	hws = clk_hw_data->hws;
 
-	for (i = 0; i < bc_data->num_clks; i++) {
-		const struct imx95_blk_ctl_clk_dev_data *data = &bc_data->clk_dev_data[i];
+	for (i = 0; i < bc->pdata->num_clks; i++) {
+		const struct imx95_blk_ctl_clk_dev_data *data = &bc->pdata->clk_dev_data[i];
 		void __iomem *reg = base + data->reg;
 
 		if (data->type == CLK_MUX) {
@@ -439,7 +439,7 @@ static int imx95_bc_probe(struct platform_device *pdev)
 	return 0;
 
 cleanup:
-	for (i = 0; i < bc_data->num_clks; i++) {
+	for (i = 0; i < bc->pdata->num_clks; i++) {
 		if (IS_ERR_OR_NULL(hws[i]))
 			continue;
 		clk_hw_unregister(hws[i]);
@@ -469,14 +469,9 @@ static int imx95_bc_runtime_resume(struct device *dev)
 static int imx95_bc_suspend(struct device *dev)
 {
 	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
-	const struct imx95_blk_ctl_dev_data *bc_data;
 	int ret;
 
-	bc_data = of_device_get_match_data(dev);
-	if (!bc_data)
-		return 0;
-
-	if (bc_data->rpm_enabled) {
+	if (bc->pdata->rpm_enabled) {
 		ret = pm_runtime_get_sync(bc->dev);
 		if (ret < 0) {
 			pm_runtime_put_noidle(bc->dev);
@@ -484,7 +479,7 @@ static int imx95_bc_suspend(struct device *dev)
 		}
 	}
 
-	bc->clk_reg_restore = readl(bc->base + bc_data->clk_reg_offset);
+	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
 
 	return 0;
 }
@@ -492,15 +487,10 @@ static int imx95_bc_suspend(struct device *dev)
 static int imx95_bc_resume(struct device *dev)
 {
 	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
-	const struct imx95_blk_ctl_dev_data *bc_data;
-
-	bc_data = of_device_get_match_data(dev);
-	if (!bc_data)
-		return 0;
 
-	writel(bc->clk_reg_restore, bc->base + bc_data->clk_reg_offset);
+	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
 
-	if (bc_data->rpm_enabled)
+	if (bc->pdata->rpm_enabled)
 		pm_runtime_put(bc->dev);
 
 	return 0;
-- 
2.34.1

[PATCH v2 02/10] clk: imx95-blk-ctl: Save/restore registers when RPM routines are called

[Laurentiu Palcu]

If runtime PM is used for the clock providers and they're part of a
power domain, then the power domain supply will be cut off when runtime
suspended. That means all BLK CTL registers belonging to that power
domain will be reset. Save the registers, then, before entering suspend
and restore them in resume.

Also, fix the suspend/resume routines and make sure we disable/enable
the clock correctly.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/clk/imx/clk-imx95-blk-ctl.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
index c72debaf3a60b..3f6bcc33bbe99 100644
--- a/drivers/clk/imx/clk-imx95-blk-ctl.c
+++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
@@ -453,7 +453,9 @@ static int imx95_bc_runtime_suspend(struct device *dev)
 {
 	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
 
+	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
 	clk_disable_unprepare(bc->clk_apb);
+
 	return 0;
 }
 
@@ -461,7 +463,10 @@ static int imx95_bc_runtime_resume(struct device *dev)
 {
 	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
 
-	return clk_prepare_enable(bc->clk_apb);
+	clk_prepare_enable(bc->clk_apb);
+	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
+
+	return 0;
 }
 #endif
 
@@ -469,17 +474,12 @@ static int imx95_bc_runtime_resume(struct device *dev)
 static int imx95_bc_suspend(struct device *dev)
 {
 	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
-	int ret;
 
-	if (bc->pdata->rpm_enabled) {
-		ret = pm_runtime_get_sync(bc->dev);
-		if (ret < 0) {
-			pm_runtime_put_noidle(bc->dev);
-			return ret;
-		}
-	}
+	if (pm_runtime_suspended(dev))
+		return 0;
 
 	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
+	clk_disable_unprepare(bc->clk_apb);
 
 	return 0;
 }
@@ -488,10 +488,11 @@ static int imx95_bc_resume(struct device *dev)
 {
 	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
 
-	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
+	if (pm_runtime_suspended(dev))
+		return 0;
 
-	if (bc->pdata->rpm_enabled)
-		pm_runtime_put(bc->dev);
+	clk_prepare_enable(bc->clk_apb);
+	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH v2 01/10] clk: imx95-blk-ctl: Save platform data in imx95_blk_ctl structure

[Abel Vesa]

On 25-07-16 11:15:05, Laurentiu Palcu wrote:
> Currently, besides probe(), the platform data is read in both suspend()
> and resume(). Let's avoid this by making pdata a member of imx95_blk_ctl
> structure.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH v2 02/10] clk: imx95-blk-ctl: Save/restore registers when RPM routines are called

[Abel Vesa]

On 25-07-16 11:15:06, Laurentiu Palcu wrote:
> If runtime PM is used for the clock providers and they're part of a
> power domain, then the power domain supply will be cut off when runtime
> suspended. That means all BLK CTL registers belonging to that power
> domain will be reset. Save the registers, then, before entering suspend
> and restore them in resume.
> 
> Also, fix the suspend/resume routines and make sure we disable/enable
> the clock correctly.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH v2 01/10] clk: imx95-blk-ctl: Save platform data in imx95_blk_ctl structure

[Frank Li]

On Wed, Jul 16, 2025 at 11:15:05AM +0300, Laurentiu Palcu wrote:
> Currently, besides probe(), the platform data is read in both suspend()
> and resume(). Let's avoid this by making pdata a member of imx95_blk_ctl
> structure.

suggested commit message.

Add a platform data (pdata) member to struct imx95_blk_ctl to store the
result of of_device_get_match_data() during probe to avoid redundant calls
in suspend and resume functions.

Frank
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  drivers/clk/imx/clk-imx95-blk-ctl.c | 36 +++++++++++------------------
>  1 file changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
> index 7e88877a62451..c72debaf3a60b 100644
> --- a/drivers/clk/imx/clk-imx95-blk-ctl.c
> +++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
> @@ -36,6 +36,7 @@ struct imx95_blk_ctl {
>  	void __iomem *base;
>  	/* clock gate register */
>  	u32 clk_reg_restore;
> +	const struct imx95_blk_ctl_dev_data *pdata;
>  };
>
>  struct imx95_blk_ctl_clk_dev_data {
> @@ -349,7 +350,6 @@ static const struct imx95_blk_ctl_dev_data imx94_dispmix_csr_dev_data = {
>  static int imx95_bc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	const struct imx95_blk_ctl_dev_data *bc_data;
>  	struct imx95_blk_ctl *bc;
>  	struct clk_hw_onecell_data *clk_hw_data;
>  	struct clk_hw **hws;
> @@ -379,25 +379,25 @@ static int imx95_bc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>
> -	bc_data = of_device_get_match_data(dev);
> -	if (!bc_data)
> +	bc->pdata = of_device_get_match_data(dev);
> +	if (!bc->pdata)
>  		return devm_of_platform_populate(dev);
>
> -	clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, bc_data->num_clks),
> +	clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, bc->pdata->num_clks),
>  				   GFP_KERNEL);
>  	if (!clk_hw_data)
>  		return -ENOMEM;
>
> -	if (bc_data->rpm_enabled) {
> +	if (bc->pdata->rpm_enabled) {
>  		devm_pm_runtime_enable(&pdev->dev);
>  		pm_runtime_resume_and_get(&pdev->dev);
>  	}
>
> -	clk_hw_data->num = bc_data->num_clks;
> +	clk_hw_data->num = bc->pdata->num_clks;
>  	hws = clk_hw_data->hws;
>
> -	for (i = 0; i < bc_data->num_clks; i++) {
> -		const struct imx95_blk_ctl_clk_dev_data *data = &bc_data->clk_dev_data[i];
> +	for (i = 0; i < bc->pdata->num_clks; i++) {
> +		const struct imx95_blk_ctl_clk_dev_data *data = &bc->pdata->clk_dev_data[i];
>  		void __iomem *reg = base + data->reg;
>
>  		if (data->type == CLK_MUX) {
> @@ -439,7 +439,7 @@ static int imx95_bc_probe(struct platform_device *pdev)
>  	return 0;
>
>  cleanup:
> -	for (i = 0; i < bc_data->num_clks; i++) {
> +	for (i = 0; i < bc->pdata->num_clks; i++) {
>  		if (IS_ERR_OR_NULL(hws[i]))
>  			continue;
>  		clk_hw_unregister(hws[i]);
> @@ -469,14 +469,9 @@ static int imx95_bc_runtime_resume(struct device *dev)
>  static int imx95_bc_suspend(struct device *dev)
>  {
>  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> -	const struct imx95_blk_ctl_dev_data *bc_data;
>  	int ret;
>
> -	bc_data = of_device_get_match_data(dev);
> -	if (!bc_data)
> -		return 0;
> -
> -	if (bc_data->rpm_enabled) {
> +	if (bc->pdata->rpm_enabled) {
>  		ret = pm_runtime_get_sync(bc->dev);
>  		if (ret < 0) {
>  			pm_runtime_put_noidle(bc->dev);
> @@ -484,7 +479,7 @@ static int imx95_bc_suspend(struct device *dev)
>  		}
>  	}
>
> -	bc->clk_reg_restore = readl(bc->base + bc_data->clk_reg_offset);
> +	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
>
>  	return 0;
>  }
> @@ -492,15 +487,10 @@ static int imx95_bc_suspend(struct device *dev)
>  static int imx95_bc_resume(struct device *dev)
>  {
>  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> -	const struct imx95_blk_ctl_dev_data *bc_data;
> -
> -	bc_data = of_device_get_match_data(dev);
> -	if (!bc_data)
> -		return 0;
>
> -	writel(bc->clk_reg_restore, bc->base + bc_data->clk_reg_offset);
> +	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
>
> -	if (bc_data->rpm_enabled)
> +	if (bc->pdata->rpm_enabled)
>  		pm_runtime_put(bc->dev);
>
>  	return 0;
> --
> 2.34.1
>

Re: [PATCH v2 02/10] clk: imx95-blk-ctl: Save/restore registers when RPM routines are called

[Frank Li]

On Wed, Jul 16, 2025 at 11:15:06AM +0300, Laurentiu Palcu wrote:
> If runtime PM is used for the clock providers and they're part of a
> power domain, then the power domain supply will be cut off when runtime
> suspended. That means all BLK CTL registers belonging to that power
> domain will be reset. Save the registers, then, before entering suspend
> and restore them in resume.
>
> Also, fix the suspend/resume routines and make sure we disable/enable
> the clock correctly.
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  drivers/clk/imx/clk-imx95-blk-ctl.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
> index c72debaf3a60b..3f6bcc33bbe99 100644
> --- a/drivers/clk/imx/clk-imx95-blk-ctl.c
> +++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
> @@ -453,7 +453,9 @@ static int imx95_bc_runtime_suspend(struct device *dev)
>  {
>  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
>
> +	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
>  	clk_disable_unprepare(bc->clk_apb);
> +
>  	return 0;
>  }
>
> @@ -461,7 +463,10 @@ static int imx95_bc_runtime_resume(struct device *dev)
>  {
>  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
>
> -	return clk_prepare_enable(bc->clk_apb);
> +	clk_prepare_enable(bc->clk_apb);

Need check clk_prepare_enable()'s return value!

> +	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> +
> +	return 0;
>  }
>  #endif
>
> @@ -469,17 +474,12 @@ static int imx95_bc_runtime_resume(struct device *dev)
>  static int imx95_bc_suspend(struct device *dev)
>  {
>  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> -	int ret;
>
> -	if (bc->pdata->rpm_enabled) {
> -		ret = pm_runtime_get_sync(bc->dev);
> -		if (ret < 0) {
> -			pm_runtime_put_noidle(bc->dev);
> -			return ret;
> -		}
> -	}
> +	if (pm_runtime_suspended(dev))
> +		return 0;
>
>  	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
> +	clk_disable_unprepare(bc->clk_apb);
>
>  	return 0;
>  }
> @@ -488,10 +488,11 @@ static int imx95_bc_resume(struct device *dev)
>  {
>  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
>
> -	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> +	if (pm_runtime_suspended(dev))
> +		return 0;
>
> -	if (bc->pdata->rpm_enabled)
> -		pm_runtime_put(bc->dev);
> +	clk_prepare_enable(bc->clk_apb);
> +	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
>
>  	return 0;
>  }

Look like needn't imx95_bc_resume() and imx95_bc_suspend()

DEFINE_RUNTIME_DEV_PM_OPS() will use pm_runtime_force_suspend(), which
do similar things with above logic.

Frank


> --
> 2.34.1
>

Re: [PATCH v2 02/10] clk: imx95-blk-ctl: Save/restore registers when RPM routines are called

[Laurentiu Palcu]

Hi Frank,

On Wed, Jul 16, 2025 at 02:29:01PM -0400, Frank Li wrote:
> On Wed, Jul 16, 2025 at 11:15:06AM +0300, Laurentiu Palcu wrote:
> > If runtime PM is used for the clock providers and they're part of a
> > power domain, then the power domain supply will be cut off when runtime
> > suspended. That means all BLK CTL registers belonging to that power
> > domain will be reset. Save the registers, then, before entering suspend
> > and restore them in resume.
> >
> > Also, fix the suspend/resume routines and make sure we disable/enable
> > the clock correctly.
> >
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > ---
> >  drivers/clk/imx/clk-imx95-blk-ctl.c | 25 +++++++++++++------------
> >  1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
> > index c72debaf3a60b..3f6bcc33bbe99 100644
> > --- a/drivers/clk/imx/clk-imx95-blk-ctl.c
> > +++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
> > @@ -453,7 +453,9 @@ static int imx95_bc_runtime_suspend(struct device *dev)
> >  {
> >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> >
> > +	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
> >  	clk_disable_unprepare(bc->clk_apb);
> > +
> >  	return 0;
> >  }
> >
> > @@ -461,7 +463,10 @@ static int imx95_bc_runtime_resume(struct device *dev)
> >  {
> >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> >
> > -	return clk_prepare_enable(bc->clk_apb);
> > +	clk_prepare_enable(bc->clk_apb);
> 
> Need check clk_prepare_enable()'s return value!
> 
> > +	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> > +
> > +	return 0;
> >  }
> >  #endif
> >
> > @@ -469,17 +474,12 @@ static int imx95_bc_runtime_resume(struct device *dev)
> >  static int imx95_bc_suspend(struct device *dev)
> >  {
> >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> > -	int ret;
> >
> > -	if (bc->pdata->rpm_enabled) {
> > -		ret = pm_runtime_get_sync(bc->dev);
> > -		if (ret < 0) {
> > -			pm_runtime_put_noidle(bc->dev);
> > -			return ret;
> > -		}
> > -	}
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> >
> >  	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
> > +	clk_disable_unprepare(bc->clk_apb);
> >
> >  	return 0;
> >  }
> > @@ -488,10 +488,11 @@ static int imx95_bc_resume(struct device *dev)
> >  {
> >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> >
> > -	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> >
> > -	if (bc->pdata->rpm_enabled)
> > -		pm_runtime_put(bc->dev);
> > +	clk_prepare_enable(bc->clk_apb);
> > +	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> >
> >  	return 0;
> >  }
> 
> Look like needn't imx95_bc_resume() and imx95_bc_suspend()
> 
> DEFINE_RUNTIME_DEV_PM_OPS() will use pm_runtime_force_suspend(), which
> do similar things with above logic.

As I said for v1, we cannot use DEFINE_RUNTIME_DEV_PM_OPS(). This driver
is used for various clock providers and RPM can be disabled for some of
them (see rpm_enabled flag in platform data). When RPM is disabled and
DEFINE_RUNTIME_DEV_PM_OPS() is used, pm_runtime_force_suspend() is
called, as you pointed out, and suspend() is never called.

Thanks,
Laurentiu

> 
> Frank
> 
> 
> > --
> > 2.34.1
> >

Re: [PATCH v2 02/10] clk: imx95-blk-ctl: Save/restore registers when RPM routines are called

[Frank Li]

On Thu, Jul 17, 2025 at 03:23:38PM +0300, Laurentiu Palcu wrote:
> Hi Frank,
>
> On Wed, Jul 16, 2025 at 02:29:01PM -0400, Frank Li wrote:
> > On Wed, Jul 16, 2025 at 11:15:06AM +0300, Laurentiu Palcu wrote:
> > > If runtime PM is used for the clock providers and they're part of a
> > > power domain, then the power domain supply will be cut off when runtime
> > > suspended. That means all BLK CTL registers belonging to that power
> > > domain will be reset. Save the registers, then, before entering suspend
> > > and restore them in resume.
> > >
> > > Also, fix the suspend/resume routines and make sure we disable/enable
> > > the clock correctly.
> > >
> > > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > > ---
> > >  drivers/clk/imx/clk-imx95-blk-ctl.c | 25 +++++++++++++------------
> > >  1 file changed, 13 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
> > > index c72debaf3a60b..3f6bcc33bbe99 100644
> > > --- a/drivers/clk/imx/clk-imx95-blk-ctl.c
> > > +++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
> > > @@ -453,7 +453,9 @@ static int imx95_bc_runtime_suspend(struct device *dev)
> > >  {
> > >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> > >
> > > +	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
> > >  	clk_disable_unprepare(bc->clk_apb);
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -461,7 +463,10 @@ static int imx95_bc_runtime_resume(struct device *dev)
> > >  {
> > >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> > >
> > > -	return clk_prepare_enable(bc->clk_apb);
> > > +	clk_prepare_enable(bc->clk_apb);
> >
> > Need check clk_prepare_enable()'s return value!
> >
> > > +	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> > > +
> > > +	return 0;
> > >  }
> > >  #endif
> > >
> > > @@ -469,17 +474,12 @@ static int imx95_bc_runtime_resume(struct device *dev)
> > >  static int imx95_bc_suspend(struct device *dev)
> > >  {
> > >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> > > -	int ret;
> > >
> > > -	if (bc->pdata->rpm_enabled) {
> > > -		ret = pm_runtime_get_sync(bc->dev);
> > > -		if (ret < 0) {
> > > -			pm_runtime_put_noidle(bc->dev);
> > > -			return ret;
> > > -		}
> > > -	}
> > > +	if (pm_runtime_suspended(dev))
> > > +		return 0;
> > >
> > >  	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
> > > +	clk_disable_unprepare(bc->clk_apb);
> > >
> > >  	return 0;
> > >  }
> > > @@ -488,10 +488,11 @@ static int imx95_bc_resume(struct device *dev)
> > >  {
> > >  	struct imx95_blk_ctl *bc = dev_get_drvdata(dev);
> > >
> > > -	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> > > +	if (pm_runtime_suspended(dev))
> > > +		return 0;
> > >
> > > -	if (bc->pdata->rpm_enabled)
> > > -		pm_runtime_put(bc->dev);
> > > +	clk_prepare_enable(bc->clk_apb);
> > > +	writel(bc->clk_reg_restore, bc->base + bc->pdata->clk_reg_offset);
> > >
> > >  	return 0;
> > >  }
> >
> > Look like needn't imx95_bc_resume() and imx95_bc_suspend()
> >
> > DEFINE_RUNTIME_DEV_PM_OPS() will use pm_runtime_force_suspend(), which
> > do similar things with above logic.
>
> As I said for v1, we cannot use DEFINE_RUNTIME_DEV_PM_OPS(). This driver
> is used for various clock providers and RPM can be disabled for some of
> them (see rpm_enabled flag in platform data). When RPM is disabled and
> DEFINE_RUNTIME_DEV_PM_OPS() is used, pm_runtime_force_suspend() is
> called, as you pointed out, and suspend() is never called.

Sorry, I missed your message at v1. do you know which flag impact this?

Frank

>
> Thanks,
> Laurentiu
>
> >
> > Frank
> >
> >
> > > --
> > > 2.34.1
> > >