| Message ID | 20210829154757.784699-4-dmitry.baryshkov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 6158b94ec8070e8a75f2c9b9a78cb425b54abcf8 |
| Headers | show |
| Series | clk: qcom: use power-domain for sm8250's clock controllers | expand |
On Sun 29 Aug 08:47 PDT 2021, Dmitry Baryshkov wrote: > On sm8250 dispcc and videocc registers are powered up by the MMCX power > domain. Use runtime PM calls to make sure that required power domain is > powered on while we access clock controller's registers. > As I said on previous iterations, the clock framework will ensure that the power domain is powered up around all calls back into the clock driver, so for clocks this isn't needed. But after digging some more, the gdsc registration needs this and I don't mind keeping it on for the duration of the registration, rather than having the clock framework turn it on and off. As far as I can see the reset_controller won't actually access the hardware during registration. Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> That said, qcom_reset_{assert,deassert}() doesn't seem to be performed under a pm_runtime_get() section, so I believe that should be addressed as well. I don't see that we have a problem with this in practice as of today, but it seems worthy to correct - unless I'm just missing something. Regards, Bjorn > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/clk/qcom/dispcc-sm8250.c | 27 +++++++++++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c > index 601c7c0ba483..108dd1249b6a 100644 > --- a/drivers/clk/qcom/dispcc-sm8250.c > +++ b/drivers/clk/qcom/dispcc-sm8250.c > @@ -6,6 +6,7 @@ > #include <linux/clk-provider.h> > #include <linux/module.h> > #include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > #include <linux/regmap.h> > #include <linux/reset-controller.h> > > @@ -1226,13 +1227,31 @@ static const struct of_device_id disp_cc_sm8250_match_table[] = { > }; > MODULE_DEVICE_TABLE(of, disp_cc_sm8250_match_table); > > +static void disp_cc_sm8250_pm_runtime_disable(void *data) > +{ > + pm_runtime_disable(data); > +} > + > static int disp_cc_sm8250_probe(struct platform_device *pdev) > { > struct regmap *regmap; > + int ret; > + > + pm_runtime_enable(&pdev->dev); > + > + ret = devm_add_action_or_reset(&pdev->dev, disp_cc_sm8250_pm_runtime_disable, &pdev->dev); > + if (ret) > + return ret; > + > + ret = pm_runtime_resume_and_get(&pdev->dev); > + if (ret) > + return ret; > > regmap = qcom_cc_map(pdev, &disp_cc_sm8250_desc); > - if (IS_ERR(regmap)) > + if (IS_ERR(regmap)) { > + pm_runtime_put(&pdev->dev); > return PTR_ERR(regmap); > + } > > /* note: trion == lucid, except for the prepare() op */ > BUILD_BUG_ON(CLK_ALPHA_PLL_TYPE_TRION != CLK_ALPHA_PLL_TYPE_LUCID); > @@ -1257,7 +1276,11 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev) > /* DISP_CC_XO_CLK always-on */ > regmap_update_bits(regmap, 0x605c, BIT(0), BIT(0)); > > - return qcom_cc_really_probe(pdev, &disp_cc_sm8250_desc, regmap); > + ret = qcom_cc_really_probe(pdev, &disp_cc_sm8250_desc, regmap); > + > + pm_runtime_put(&pdev->dev); > + > + return ret; > } > > static struct platform_driver disp_cc_sm8250_driver = { > -- > 2.33.0 >
diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c index 601c7c0ba483..108dd1249b6a 100644 --- a/drivers/clk/qcom/dispcc-sm8250.c +++ b/drivers/clk/qcom/dispcc-sm8250.c @@ -6,6 +6,7 @@ #include <linux/clk-provider.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset-controller.h> @@ -1226,13 +1227,31 @@ static const struct of_device_id disp_cc_sm8250_match_table[] = { }; MODULE_DEVICE_TABLE(of, disp_cc_sm8250_match_table); +static void disp_cc_sm8250_pm_runtime_disable(void *data) +{ + pm_runtime_disable(data); +} + static int disp_cc_sm8250_probe(struct platform_device *pdev) { struct regmap *regmap; + int ret; + + pm_runtime_enable(&pdev->dev); + + ret = devm_add_action_or_reset(&pdev->dev, disp_cc_sm8250_pm_runtime_disable, &pdev->dev); + if (ret) + return ret; + + ret = pm_runtime_resume_and_get(&pdev->dev); + if (ret) + return ret; regmap = qcom_cc_map(pdev, &disp_cc_sm8250_desc); - if (IS_ERR(regmap)) + if (IS_ERR(regmap)) { + pm_runtime_put(&pdev->dev); return PTR_ERR(regmap); + } /* note: trion == lucid, except for the prepare() op */ BUILD_BUG_ON(CLK_ALPHA_PLL_TYPE_TRION != CLK_ALPHA_PLL_TYPE_LUCID); @@ -1257,7 +1276,11 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev) /* DISP_CC_XO_CLK always-on */ regmap_update_bits(regmap, 0x605c, BIT(0), BIT(0)); - return qcom_cc_really_probe(pdev, &disp_cc_sm8250_desc, regmap); + ret = qcom_cc_really_probe(pdev, &disp_cc_sm8250_desc, regmap); + + pm_runtime_put(&pdev->dev); + + return ret; } static struct platform_driver disp_cc_sm8250_driver = {
On sm8250 dispcc and videocc registers are powered up by the MMCX power domain. Use runtime PM calls to make sure that required power domain is powered on while we access clock controller's registers. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/clk/qcom/dispcc-sm8250.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)