| Message ID | 1566299605-15641-8-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: imx8: add new clock binding for better pm support | expand |
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) > +}; > +
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) > > +}; > > +
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, };
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(+)