[V4,07/11] clk: imx: scu: add suspend/resume support
diff mbox series

Message ID 1566299605-15641-8-git-send-email-aisheng.dong@nxp.com
State New
Headers show
Series
  • clk: imx8: add new clock binding for better pm support
Related show

Commit Message

Aisheng Dong Aug. 20, 2019, 11:13 a.m. UTC
Clock state will be lost when its power domain is completely off
during system suspend/resume. So we save and restore the state
accordingly in suspend/resume callback.

Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4: no changes
v3: new patch
---
 drivers/clk/imx/clk-scu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

Stephen Boyd Sept. 6, 2019, 5:09 p.m. UTC | #1
Quoting Dong Aisheng (2019-08-20 04:13:21)
> Clock state will be lost when its power domain is completely off
> during system suspend/resume. So we save and restore the state
> accordingly in suspend/resume callback.

And this doesn't need any coordination with other clks in the clk tree
right?

> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index edc39d7..8d9cfa2 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -46,6 +46,10 @@ struct clk_scu {
>         struct clk_hw hw;
>         u16 rsrc_id;
>         u8 clk_type;
> +
> +       /* for state save&restore */
> +       bool is_enabled;
> +       u32 rate;
>  };
>  
>  /*
> @@ -425,6 +429,9 @@ struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
>                 hw = ERR_PTR(ret);
>         }
>  
> +       if (dev)
> +               dev_set_drvdata(dev, clk);
> +
>         return hw;
>  }
>  
> @@ -481,10 +488,52 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
>         return 0;
>  }
>  
> +int __maybe_unused imx_clk_scu_suspend(struct device *dev)

static?

> +{
> +       struct clk_scu *clk = dev_get_drvdata(dev);
> +
> +       clk->rate = clk_hw_get_rate(&clk->hw);
> +       clk->is_enabled = clk_hw_is_enabled(&clk->hw);
> +
> +       if (clk->rate)
> +               dev_dbg(dev, "save rate %d\n", clk->rate);
> +
> +       if (clk->is_enabled)
> +               dev_dbg(dev, "save enabled state\n");
> +
> +       return 0;
> +}
> +
> +int __maybe_unused imx_clk_scu_resume(struct device *dev)

static?

> +{
> +       struct clk_scu *clk = dev_get_drvdata(dev);
> +       int ret = 0;
> +
> +       if (clk->rate) {
> +               ret = clk_scu_set_rate(&clk->hw, clk->rate, 0);
> +               dev_dbg(dev, "restore rate %d %s\n", clk->rate,
> +                       !ret ? "success" : "failed");
> +       }
> +
> +       if (clk->is_enabled) {
> +               ret = clk_scu_prepare(&clk->hw);
> +               dev_dbg(dev, "restore enabled state %s\n",
> +                       !ret ? "success" : "failed");
> +       }
> +
> +       return ret;
> +}
> +
> +const struct dev_pm_ops imx_clk_scu_pm_ops = {

static?

> +       SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_clk_scu_suspend,
> +                                     imx_clk_scu_resume)
> +};
> +
Dong Aisheng Sept. 9, 2019, 10:35 a.m. UTC | #2
On Sat, Sep 7, 2019 at 5:32 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Dong Aisheng (2019-08-20 04:13:21)
> > Clock state will be lost when its power domain is completely off
> > during system suspend/resume. So we save and restore the state
> > accordingly in suspend/resume callback.
>
> And this doesn't need any coordination with other clks in the clk tree
> right?

AFAIK no as SC firmware may have handled it properly.

>
> > diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> > index edc39d7..8d9cfa2 100644
> > --- a/drivers/clk/imx/clk-scu.c
> > +++ b/drivers/clk/imx/clk-scu.c
> > @@ -46,6 +46,10 @@ struct clk_scu {
> >         struct clk_hw hw;
> >         u16 rsrc_id;
> >         u8 clk_type;
> > +
> > +       /* for state save&restore */
> > +       bool is_enabled;
> > +       u32 rate;
> >  };
> >
> >  /*
> > @@ -425,6 +429,9 @@ struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
> >                 hw = ERR_PTR(ret);
> >         }
> >
> > +       if (dev)
> > +               dev_set_drvdata(dev, clk);
> > +
> >         return hw;
> >  }
> >
> > @@ -481,10 +488,52 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
> >         return 0;
> >  }
> >
> > +int __maybe_unused imx_clk_scu_suspend(struct device *dev)
>
> static?
>
> > +{
> > +       struct clk_scu *clk = dev_get_drvdata(dev);
> > +
> > +       clk->rate = clk_hw_get_rate(&clk->hw);
> > +       clk->is_enabled = clk_hw_is_enabled(&clk->hw);
> > +
> > +       if (clk->rate)
> > +               dev_dbg(dev, "save rate %d\n", clk->rate);
> > +
> > +       if (clk->is_enabled)
> > +               dev_dbg(dev, "save enabled state\n");
> > +
> > +       return 0;
> > +}
> > +
> > +int __maybe_unused imx_clk_scu_resume(struct device *dev)
>
> static?
>
> > +{
> > +       struct clk_scu *clk = dev_get_drvdata(dev);
> > +       int ret = 0;
> > +
> > +       if (clk->rate) {
> > +               ret = clk_scu_set_rate(&clk->hw, clk->rate, 0);
> > +               dev_dbg(dev, "restore rate %d %s\n", clk->rate,
> > +                       !ret ? "success" : "failed");
> > +       }
> > +
> > +       if (clk->is_enabled) {
> > +               ret = clk_scu_prepare(&clk->hw);
> > +               dev_dbg(dev, "restore enabled state %s\n",
> > +                       !ret ? "success" : "failed");
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +const struct dev_pm_ops imx_clk_scu_pm_ops = {
>
> static?
>

Sorry that i missed to update here as those function are changed to be
used within
this file now.
Will fix.

Regards
Aisheng

> > +       SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_clk_scu_suspend,
> > +                                     imx_clk_scu_resume)
> > +};
> > +

Patch
diff mbox series

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index edc39d7..8d9cfa2 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -46,6 +46,10 @@  struct clk_scu {
 	struct clk_hw hw;
 	u16 rsrc_id;
 	u8 clk_type;
+
+	/* for state save&restore */
+	bool is_enabled;
+	u32 rate;
 };
 
 /*
@@ -425,6 +429,9 @@  struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
 		hw = ERR_PTR(ret);
 	}
 
+	if (dev)
+		dev_set_drvdata(dev, clk);
+
 	return hw;
 }
 
@@ -481,10 +488,52 @@  static int imx_clk_scu_probe(struct platform_device *pdev)
 	return 0;
 }
 
+int __maybe_unused imx_clk_scu_suspend(struct device *dev)
+{
+	struct clk_scu *clk = dev_get_drvdata(dev);
+
+	clk->rate = clk_hw_get_rate(&clk->hw);
+	clk->is_enabled = clk_hw_is_enabled(&clk->hw);
+
+	if (clk->rate)
+		dev_dbg(dev, "save rate %d\n", clk->rate);
+
+	if (clk->is_enabled)
+		dev_dbg(dev, "save enabled state\n");
+
+	return 0;
+}
+
+int __maybe_unused imx_clk_scu_resume(struct device *dev)
+{
+	struct clk_scu *clk = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (clk->rate) {
+		ret = clk_scu_set_rate(&clk->hw, clk->rate, 0);
+		dev_dbg(dev, "restore rate %d %s\n", clk->rate,
+			!ret ? "success" : "failed");
+	}
+
+	if (clk->is_enabled) {
+		ret = clk_scu_prepare(&clk->hw);
+		dev_dbg(dev, "restore enabled state %s\n",
+			!ret ? "success" : "failed");
+	}
+
+	return ret;
+}
+
+const struct dev_pm_ops imx_clk_scu_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_clk_scu_suspend,
+				      imx_clk_scu_resume)
+};
+
 static struct platform_driver imx_clk_scu_driver = {
 	.driver = {
 		.name = "imx-scu-clk",
 		.suppress_bind_attrs = true,
+		.pm = &imx_clk_scu_pm_ops,
 	},
 	.probe = imx_clk_scu_probe,
 };