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 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); + 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);
@@ -57,6 +80,8 @@ static int clk_gfm_set_parent(struct clk_hw *hw, u8 index)
writel(val, clk->gfm_mux);
+ pm_runtime_mark_last_busy(clk->priv->dev); + return 0; }
@@ -251,6 +276,8 @@ static int lpass_gfm_clk_driver_probe(struct platform_device *pdev) if (IS_ERR(cc->base)) return PTR_ERR(cc->base);
+ cc->dev = dev; + err = devm_pm_runtime_enable(dev); if (err) return err;
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?
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?
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);
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);
Quoting Srinivas Kandagatla (2023-03-21 13:33:49)
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?
Please add a Link: after the reported-by and use a lore link instead of spinics please.
{ 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
The get_parent() clk op is called from a couple places in the clk framework. I guess that you're getting called from clk_core_reparent_orphans() where the runtime PM count isn't incremented? Can you confirm? Either way, this should be fixed in the framework and not in the driver.
On Tue, 21 Mar 2023 at 23:28, Srinivas Kandagatla srinivas.kandagatla@linaro.org wrote:
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.
Thank you Srini, this fixes the boot regression I see on the RB5 booting v6.1.y and v6.3-rc kernel versions.
Tested-by: Amit Pundir amit.pundir@linaro.org
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 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);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);@@ -57,6 +80,8 @@ static int clk_gfm_set_parent(struct clk_hw *hw, u8 index)
writel(val, clk->gfm_mux);
pm_runtime_mark_last_busy(clk->priv->dev);return 0;}
@@ -251,6 +276,8 @@ static int lpass_gfm_clk_driver_probe(struct platform_device *pdev) if (IS_ERR(cc->base)) return PTR_ERR(cc->base);
cc->dev = dev;err = devm_pm_runtime_enable(dev); if (err) return err;-- 2.21.0
linux-stable-mirror@lists.linaro.org
-
Amit Pundir -
Srinivas Kandagatla -
Stephen Boyd