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.pa...@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 > >