Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 21 Mar 2023 20:33:49 +0000 | Subject | Re: [PATCH] clk: qcom: gfm-mux: use runtime pm while accessing registers | From | Srinivas Kandagatla <> |
| |
On 21/03/2023 18:46, Stephen Boyd wrote: > Quoting Srinivas Kandagatla (2023-03-21 10:57:58) >> gfm mux driver does support runtime pm but we never use it while >> accessing registers. Looks like this driver was getting lucky and >> totally depending on other drivers to leave the clk on. >> >> Fix this by doing runtime pm while accessing registers. >> >> Fixes: a2d8f507803e ("clk: qcom: Add support to LPASS AUDIO_CC Glitch Free Mux clocks") >> Cc: stable@vger.kernel.org >> Reported-by: Amit Pundir <amit.pundir@linaro.org> > > Is there a link to the report?
https://www.spinics.net/lists/stable/msg638380.html
> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/clk/qcom/lpass-gfm-sm8250.c | 29 ++++++++++++++++++++++++++++- >> 1 file changed, 28 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/clk/qcom/lpass-gfm-sm8250.c b/drivers/clk/qcom/lpass-gfm-sm8250.c >> index 96f476f24eb2..bcf0ea534f7f 100644 >> --- a/drivers/clk/qcom/lpass-gfm-sm8250.c >> +++ b/drivers/clk/qcom/lpass-gfm-sm8250.c >> @@ -38,14 +38,37 @@ struct clk_gfm { >> static u8 clk_gfm_get_parent(struct clk_hw *hw) >> { >> struct clk_gfm *clk = to_clk_gfm(hw); >> + int ret; >> + u8 parent; >> + >> + ret = pm_runtime_resume_and_get(clk->priv->dev); >> + if (ret < 0 && ret != -EACCES) { >> + dev_err_ratelimited(clk->priv->dev, >> + "pm_runtime_resume_and_get failed in %s, ret %d\n", >> + __func__, ret); >> + return ret; >> + } >> + >> + parent = readl(clk->gfm_mux) & clk->mux_mask; >> + >> + pm_runtime_mark_last_busy(clk->priv->dev); >> >> - return readl(clk->gfm_mux) & clk->mux_mask; >> + return parent; >> } >> >> static int clk_gfm_set_parent(struct clk_hw *hw, u8 index) >> { >> struct clk_gfm *clk = to_clk_gfm(hw); >> unsigned int val; >> + int ret; >> + >> + ret = pm_runtime_resume_and_get(clk->priv->dev); > > Doesn't the clk framework already do this? Why do we need to do it > again?
You are right, clk core already does do pm_runtime_resume_and_get for set_parent.
this looks redundant here.
so we need only need to add this for get_parent
--srini > >> + if (ret < 0 && ret != -EACCES) { >> + dev_err_ratelimited(clk->priv->dev, >> + "pm_runtime_resume_and_get failed in %s, ret %d\n", >> + __func__, ret); >> + return ret; >> + } >> >> val = readl(clk->gfm_mux); >>
| ![\](/images/icornerr.gif) |